Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: revert specs #217

Merged
merged 115 commits into from
Jul 19, 2024
Merged

feat: revert specs #217

merged 115 commits into from
Jul 19, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Jul 11, 2024

implements specs for revert

NOTE: unit tests and worker will be in follow up PR once unit test PR is merged and specs are approved

Qs:

  • revert seems to be shadowing built in name, what can be different naming here?

Summary by CodeRabbit

  • New Features

    • Introduced a new event WithdrawAndRevert to track specific withdrawal operations.
    • Added functions withdrawAndRevert, executeRevert, and revertWithERC20 for improved token handling.
    • Enhanced interfaces with new events and functions for better transaction tracking.
    • Added a new section in the README regarding ongoing development of Version 2.
  • Bug Fixes

    • Updated event signatures to improve clarity and trackability of transactions.
  • Chores

    • Introduced a new .gitattributes file for better repository management.
    • Updated workflow for NPM publishing to support submodules and toolchain installation.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 9.09091% with 20 lines in your changes missing coverage. Please review.

Project coverage is 47.13%. Comparing base (deee4d3) to head (78b292d).

Files Patch % Lines
contracts/prototypes/evm/GatewayEVM.sol 0.00% 8 Missing ⚠️
contracts/prototypes/zevm/GatewayZEVM.sol 22.22% 7 Missing ⚠️
contracts/prototypes/evm/ERC20CustodyNew.sol 0.00% 3 Missing ⚠️
contracts/prototypes/zevm/TestZContract.sol 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##           zeta-connector     #217      +/-   ##
==================================================
- Coverage           50.16%   47.13%   -3.04%     
==================================================
  Files                  12       12              
  Lines                 295      314      +19     
  Branches               82       87       +5     
==================================================
  Hits                  148      148              
- Misses                147      166      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

contracts/prototypes/evm/GatewayEVM.sol Outdated Show resolved Hide resolved
contracts/prototypes/zevm/GatewayZEVM.sol Outdated Show resolved Hide resolved
@@ -7,6 +7,13 @@ struct zContext {
uint256 chainID;
}

// TODO: define revertContext
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not planned for this PR? Would need issue otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is same as zContext atm, was not sure if this information is enough, or we should wait for integration with zetacore to see if we need more info (this applies to zContext as well)

contracts/prototypes/evm/ERC20CustodyNew.sol Show resolved Hide resolved
contracts/prototypes/evm/GatewayEVM.sol Outdated Show resolved Hide resolved
contracts/prototypes/evm/GatewayEVM.sol Outdated Show resolved Hide resolved
contracts/prototypes/evm/interfaces.sol Outdated Show resolved Hide resolved
contracts/prototypes/evm/GatewayEVM.sol Show resolved Hide resolved
contracts/prototypes/evm/GatewayEVM.sol Show resolved Hide resolved
contracts/prototypes/zevm/GatewayZEVM.sol Show resolved Hide resolved
contracts/prototypes/zevm/TestZContract.sol Outdated Show resolved Hide resolved
contracts/prototypes/evm/GatewayEVM.sol Outdated Show resolved Hide resolved
contracts/prototypes/evm/ERC20CustodyNew.sol Show resolved Hide resolved
@skosito
Copy link
Contributor Author

skosito commented Jul 19, 2024

@lumtis @fbac i think all comments are addressed

@fbac comments about rbac and checking erc20 contract are planned after we have basic implementations done, since they span all prototypes contracts, but these will be addressed soon:

@skosito skosito requested review from lumtis and fbac July 19, 2024 00:06
Base automatically changed from zeta-connector to main July 19, 2024 15:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (8)
testFoundry/GatewayEVM.t.sol (8)

10-10: Add documentation for new imports.

Consider adding comments to explain the purpose of the new imports.


15-16: Group related imports together.

The imports for IGatewayEVM and IReceiverEVM should be grouped with the other interface imports for consistency.

-import "contracts/prototypes/evm/IGatewayEVM.sol";
-import "contracts/prototypes/evm/IReceiverEVM.sol";

26-28: Add comments for new state variables.

Consider adding comments to explain the purpose of the new state variables zetaConnector and zeta.

+ // Connector for non-native Zeta interactions
+ ZetaConnectorNonNative zetaConnector;
+ // ERC20 token for Zeta
+ TestERC20 zeta;

42-43: Add comments for token initialization.

Consider adding comments to explain the initialization of the zeta token.

+ // Initialize Zeta token
zeta = new TestERC20("zeta", "ZETA");

50-50: Add comments for connector initialization.

Consider adding comments to explain the initialization of the zetaConnector.

+ // Initialize ZetaConnectorNonNative with gateway and zeta token addresses
zetaConnector = new ZetaConnectorNonNative(address(gateway), address(zeta));

184-186: Add comments for new state variables.

Consider adding comments to explain the purpose of the new state variables zetaConnector and zeta.

+ // Connector for non-native Zeta interactions
+ ZetaConnectorNonNative zetaConnector;
+ // ERC20 token for Zeta
+ TestERC20 zeta;

199-200: Add comments for token initialization.

Consider adding comments to explain the initialization of the zeta token.

+ // Initialize Zeta token
zeta = new TestERC20("zeta", "ZETA");

206-206: Add comments for connector initialization.

Consider adding comments to explain the initialization of the zetaConnector.

+ // Initialize ZetaConnectorNonNative with gateway and zeta token addresses
zetaConnector = new ZetaConnectorNonNative(address(gateway), address(zeta));
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0df7cef and 2ed3763.

Files ignored due to path filters (103)
  • pkg/contracts/prototypes/evm/erc20custodynew.sol/erc20custodynew.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/gatewayevm.sol/gatewayevm.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/gatewayevmupgradetest.sol/gatewayevmupgradetest.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/igatewayevm.sol/igatewayevm.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/igatewayevm.sol/igatewayevmerrors.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/igatewayevm.sol/igatewayevmevents.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/igatewayevm.sol/revertable.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/ireceiverevm.sol/ireceiverevmevents.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/izetanonethnew.sol/izetanonethnew.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/zetaconnectornative.sol/zetaconnectornative.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/zetaconnectornewbase.sol/zetaconnectornewbase.go is excluded by !pkg/**
  • pkg/contracts/prototypes/evm/zetaconnectornonnative.sol/zetaconnectornonnative.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/gatewayzevm.sol/gatewayzevm.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/igatewayzevm.sol/igatewayzevm.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/igatewayzevm.sol/igatewayzevmerrors.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/igatewayzevm.sol/igatewayzevmevents.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/senderzevm.sol/senderzevm.go is excluded by !pkg/**
  • pkg/contracts/prototypes/zevm/testzcontract.sol/testzcontract.go is excluded by !pkg/**
  • pkg/contracts/zevm/interfaces/zcontract.sol/universalcontract.go is excluded by !pkg/**
  • pkg/contracts/zevm/systemcontract.sol/systemcontract.go is excluded by !pkg/**
  • pkg/contracts/zevm/testing/systemcontractmock.sol/systemcontractmock.go is excluded by !pkg/**
  • pkg/openzeppelin/contracts-upgradeable/security/reentrancyguardupgradeable.sol/reentrancyguardupgradeable.go is excluded by !pkg/**
  • typechain-types/@openzeppelin/contracts-upgradeable/index.ts is excluded by !typechain-types/**
  • typechain-types/@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.ts is excluded by !typechain-types/**
  • typechain-types/@openzeppelin/contracts-upgradeable/security/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/ERC20CustodyNew.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/GatewayEVM.sol/GatewayEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/GatewayEVM.sol/Revertable.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/GatewayEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/GatewayEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/GatewayEVMUpgradeTest.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVMErrors.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVMEvents.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IGatewayEVM.sol/Revertable.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IGatewayEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IReceiverEVM.sol/IReceiverEVMEvents.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IReceiverEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/IZetaNonEthNew.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/ZetaConnectorNative.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/ZetaConnectorNew.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/ZetaConnectorNewBase.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/ZetaConnectorNonNative.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/interfaces.sol/IGatewayEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/evm/interfaces.sol/IGatewayEVMEvents.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/test/GatewayEVM.t.sol/GatewayEVMTest.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/test/GatewayEVMZEVM.t.sol/GatewayEVMZEVMTest.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/GatewayZEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVM.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVMErrors.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVMEvents.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/IGatewayZEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/TestZContract.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/prototypes/zevm/interfaces.sol/IGatewayZEVMEvents.ts is excluded by !typechain-types/**
  • typechain-types/contracts/zevm/interfaces/index.ts is excluded by !typechain-types/**
  • typechain-types/contracts/zevm/interfaces/zContract.sol/UniversalContract.ts is excluded by !typechain-types/**
  • typechain-types/contracts/zevm/interfaces/zContract.sol/ZContract.ts is excluded by !typechain-types/**
  • typechain-types/contracts/zevm/interfaces/zContract.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/@openzeppelin/contracts-upgradeable/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/@openzeppelin/contracts-upgradeable/security/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/ERC20CustodyNew__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/GatewayEVM.sol/GatewayEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/GatewayEVM.sol/Revertable__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/GatewayEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/GatewayEVMUpgradeTest__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/GatewayEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVMErrors__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVMEvents__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IGatewayEVM.sol/IGatewayEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IGatewayEVM.sol/Revertable__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IGatewayEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IReceiverEVM.sol/IReceiverEVMEvents__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IReceiverEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/IZetaNonEthNew__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/ZetaConnectorNative__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/ZetaConnectorNewBase__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/ZetaConnectorNonNative__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/interfaces.sol/IGatewayEVMEvents__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/evm/interfaces.sol/IGatewayEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/test/GatewayEVM.t.sol/GatewayEVMTest__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/test/GatewayEVMZEVM.t.sol/GatewayEVMZEVMTest__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/GatewayZEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVMErrors__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVMEvents__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/IGatewayZEVM.sol/IGatewayZEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/IGatewayZEVM.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/SenderZEVM__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/TestZContract__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/interfaces.sol/IGatewayZEVMErrors__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/prototypes/zevm/interfaces.sol/IGatewayZEVMEvents__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/SystemContract.sol/SystemContract__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/interfaces/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/interfaces/zContract.sol/UniversalContract__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/interfaces/zContract.sol/ZContract__factory.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/interfaces/zContract.sol/index.ts is excluded by !typechain-types/**
  • typechain-types/factories/contracts/zevm/testing/SystemContractMock.sol/SystemContractMock__factory.ts is excluded by !typechain-types/**
  • typechain-types/hardhat.d.ts is excluded by !typechain-types/**
  • typechain-types/index.ts is excluded by !typechain-types/**
Files selected for processing (27)
  • .gitattributes (1 hunks)
  • .github/workflows/publish-npm.yaml (1 hunks)
  • contracts/prototypes/evm/ERC20CustodyNew.sol (3 hunks)
  • contracts/prototypes/evm/GatewayEVM.sol (7 hunks)
  • contracts/prototypes/evm/GatewayEVMUpgradeTest.sol (2 hunks)
  • contracts/prototypes/evm/IGatewayEVM.sol (2 hunks)
  • contracts/prototypes/evm/IReceiverEVM.sol (1 hunks)
  • contracts/prototypes/evm/IZetaNonEthNew.sol (1 hunks)
  • contracts/prototypes/evm/ZetaConnectorNative.sol (1 hunks)
  • contracts/prototypes/evm/ZetaConnectorNewBase.sol (1 hunks)
  • contracts/prototypes/evm/ZetaConnectorNonNative.sol (1 hunks)
  • contracts/prototypes/zevm/GatewayZEVM.sol (4 hunks)
  • contracts/prototypes/zevm/IGatewayZEVM.sol (2 hunks)
  • contracts/prototypes/zevm/SenderZEVM.sol (1 hunks)
  • contracts/prototypes/zevm/TestZContract.sol (2 hunks)
  • contracts/zevm/interfaces/zContract.sol (2 hunks)
  • foundry.toml (1 hunks)
  • readme.md (1 hunks)
  • scripts/worker.ts (1 hunks)
  • slither.config.json (1 hunks)
  • test/fuzz/ERC20CustodyNewEchidnaTest.sol (1 hunks)
  • test/fuzz/GatewayEVMEchidnaTest.sol (1 hunks)
  • test/prototypes/GatewayEVMUniswap.spec.ts (1 hunks)
  • testFoundry/GatewayEVM.t.sol (6 hunks)
  • testFoundry/GatewayEVMUpgrade.t.sol (3 hunks)
  • testFoundry/GatewayEVMZEVM.t.sol (5 hunks)
  • testFoundry/GatewayZEVM.t.sol (3 hunks)
Files skipped from review due to trivial changes (2)
  • .gitattributes
  • foundry.toml
Additional context used
Path-based instructions (18)
contracts/prototypes/evm/IZetaNonEthNew.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/IReceiverEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/zevm/interfaces/zContract.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

test/fuzz/GatewayEVMEchidnaTest.sol (1)

Pattern test/**: Review the test files for proper coverage, edge cases, and best practices.

contracts/prototypes/zevm/TestZContract.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/ZetaConnectorNewBase.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/zevm/SenderZEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/zevm/IGatewayZEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/IGatewayEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/ZetaConnectorNonNative.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

test/fuzz/ERC20CustodyNewEchidnaTest.sol (1)

Pattern test/**: Review the test files for proper coverage, edge cases, and best practices.

contracts/prototypes/evm/ZetaConnectorNative.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/ERC20CustodyNew.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

test/prototypes/GatewayEVMUniswap.spec.ts (1)

Pattern test/**: Review the test files for proper coverage, edge cases, and best practices.

scripts/worker.ts (1)

Pattern scripts/**: Review the Hardhat scripts for best practices.

contracts/prototypes/zevm/GatewayZEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/GatewayEVM.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

contracts/prototypes/evm/GatewayEVMUpgradeTest.sol (1)

Pattern contracts/**: Review the Solidity contracts for security vulnerabilities and best practices.

Additional comments not posted (88)
slither.config.json (1)

3-3: Confirm the intention behind excluding contracts/evm and contracts/zevm.

The addition of these paths to filter_paths will exclude them from Slither's analysis. Ensure this exclusion is intentional and doesn't omit critical security checks.

contracts/prototypes/evm/IZetaNonEthNew.sol (1)

1-13: Ensure thorough testing of burnFrom and mint functions.

The burnFrom and mint functions should be thoroughly tested to ensure they handle edge cases and adhere to security best practices, such as preventing unauthorized minting or burning.

contracts/prototypes/evm/IReceiverEVM.sol (1)

1-9: Ensure event parameters are well-documented and tested.

The events defined in the interface should have clear documentation for each parameter. Ensure that these events are thoroughly tested to verify that they are emitted correctly in all relevant scenarios.

contracts/zevm/interfaces/zContract.sol (3)

11-15: LGTM! New struct revertContext is well-defined.

The new struct follows a clear and consistent pattern.


35-40: LGTM! New function onRevert is well-defined.

The function signature is consistent and follows the existing pattern.


27-32: LGTM! New function onCrossChainCall is well-defined.

The function signature is consistent and follows the existing pattern.

.github/workflows/publish-npm.yaml (2)

14-15: LGTM! The new step for handling submodules is well-defined.

Including submodules: recursive ensures that any submodules are also checked out during the repository cloning process.


23-25: LGTM! The new step for installing the Foundry toolchain is well-defined.

This step is crucial for projects that depend on the Foundry toolchain, allowing for a more streamlined setup of the development environment.

contracts/prototypes/zevm/TestZContract.sol (3)

7-7: Ensure compatibility with UniversalContract.

The class now inherits from UniversalContract instead of zContract. Verify that UniversalContract provides the necessary functionality and does not introduce breaking changes.


16-20: Improved handling of message parameter.

The function now includes a conditional check for the message parameter, ensuring it is only decoded if it contains data. This prevents potential errors and improves robustness.


29-32: Improved handling of message parameter.

The function now includes a conditional check for the message parameter, ensuring it is only decoded if it contains data. This prevents potential errors and improves robustness.

contracts/prototypes/evm/ZetaConnectorNewBase.sol (3)

14-15: Ensure proper initialization of immutable variables.

The contract declares gateway and zetaToken as immutable variables. Verify that these variables are correctly initialized in the constructor.


20-26: Check for zero address in constructor.

The constructor includes a check to ensure that neither _gateway nor _zetaToken are zero addresses. This is a good practice to prevent errors related to invalid addresses.


28-32: Implement abstract functions in derived contracts.

The contract declares three abstract functions: withdraw, withdrawAndCall, and receiveTokens. Ensure that these functions are implemented in derived contracts.

contracts/prototypes/zevm/SenderZEVM.sol (1)

5-5: Verify new interface dependency.

The import statement has been changed from interfaces.sol to IGatewayZEVM.sol. Ensure that the new interface provides the necessary functionality and does not introduce breaking changes.

contracts/prototypes/zevm/IGatewayZEVM.sol (2)

49-49: LGTM! The new error enhances error handling capabilities.

The addition of FailedZetaSent improves the precision of error feedback.


38-38: Verify the impact of the updated Withdrawal event signature.

The Withdrawal event signature has been updated to include an additional parameter address zrc20. Ensure that all code listening for this event is updated accordingly.

contracts/prototypes/evm/IGatewayEVM.sol (3)

41-43: LGTM! The new function enhances error handling and state recovery.

The addition of onRevert improves the contract's capability to manage error handling and state recovery.


29-39: Verify the impact of the modified executeWithERC20 function signature and the new revertWithERC20 function.

The executeWithERC20 function signature has been modified to remove its return type. Ensure that all calls to this function handle the lack of return data appropriately. Additionally, verify the usage of the new revertWithERC20 function in the codebase.


6-8: Verify the impact of the new Reverted and RevertedWithERC20 events.

The new events Reverted and RevertedWithERC20 have been added. Ensure that all code listening for these events is updated accordingly.

Verification successful

The new events Reverted and RevertedWithERC20 are integrated into the codebase.

The search results confirm multiple instances where these events are referenced, emitted, and filtered. The integration appears to be correctly handled.

  • Files with references:
    • contracts/prototypes/evm/IGatewayEVM.sol
    • contracts/prototypes/evm/GatewayEVM.sol
    • contracts/prototypes/test/GatewayEVMZEVM.t.sol
    • contracts/prototypes/test/GatewayEVM.t.sol
    • contracts/prototypes/evm/interfaces.sol
    • pkg/contracts/prototypes/evm/igatewayevm.sol
    • pkg/contracts/prototypes/evm/gatewayevmupgradetest.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new `Reverted` and `RevertedWithERC20` events in the codebase.

# Test: Search for the event usage. Expect: Only occurrences of the new events.
rg --type solidity -A 5 $'Reverted(address indexed destination, uint256 value, bytes data)'
rg --type solidity -A 5 $'RevertedWithERC20(address indexed token, address indexed to, uint256 amount, bytes data)'

Length of output: 275


Script:

#!/bin/bash
# Description: Verify the usage of the new `Reverted` and `RevertedWithERC20` events in the codebase.

# Find all Solidity files and then search for the event usage within those files.
fd -e sol -x rg 'Reverted(address indexed destination, uint256 value, bytes data)' {}
fd -e sol -x rg 'RevertedWithERC20(address indexed token, address indexed to, uint256 amount, bytes data)' {}

Length of output: 203


Script:

#!/bin/bash
# Description: Verify the usage of the new `Reverted` and `RevertedWithERC20` events in the codebase.

# Find all Solidity files and then search for the event names within those files.
fd -e sol -x rg 'Reverted' {}
fd -e sol -x rg 'RevertedWithERC20' {}

Length of output: 161191

contracts/prototypes/evm/ZetaConnectorNonNative.sol (4)

9-11: LGTM! The constructor correctly initializes the contract.

The constructor initializes the contract by calling the parent constructor with _gateway and _zetaToken parameters.


30-34: LGTM! The function correctly implements the token transfer and burn logic.

The receiveTokens function correctly implements the logic to handle token transfer and burn them.


20-27: Verify the usage of the nonReentrant modifier.

The withdrawAndCall function correctly implements the withdrawal and call logic. Ensure that the nonReentrant modifier is used appropriately to prevent reentrancy attacks.

Verification successful

Verified the usage of the nonReentrant modifier.

The withdrawAndCall function correctly implements the nonReentrant modifier to prevent reentrancy attacks. The usage is consistent with other functions in the codebase, ensuring security against such vulnerabilities.

  • contracts/prototypes/evm/ZetaConnectorNonNative.sol
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `nonReentrant` modifier in the codebase.

# Test: Search for the `nonReentrant` modifier usage. Expect: Appropriate usage to prevent reentrancy attacks.
rg --type solidity -A 5 $'nonReentrant'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify the usage of the `nonReentrant` modifier in Solidity files.

# Test: Search for the `nonReentrant` modifier usage. Expect: Appropriate usage to prevent reentrancy attacks.
rg 'nonReentrant' --glob '*.sol' -A 5

Length of output: 14249


14-17: Verify the usage of the nonReentrant modifier.

The withdraw function correctly implements the withdrawal logic. Ensure that the nonReentrant modifier is used appropriately to prevent reentrancy attacks.

Verification successful

Verification successful.

The nonReentrant modifier is correctly implemented and used in the withdraw function to prevent reentrancy attacks.

  • The modifier is implemented to set a _locked flag and revert if the function is re-entered.
  • This pattern is a standard and effective way to prevent reentrancy attacks.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `nonReentrant` modifier in the codebase.

# Test: Search for the `nonReentrant` modifier usage. Expect: Appropriate usage to prevent reentrancy attacks.
rg --type solidity -A 5 $'nonReentrant'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify the usage of the `nonReentrant` modifier in Solidity files.

# Find all Solidity files and search for the `nonReentrant` modifier usage.
fd -e sol -x grep -H -A 5 'nonReentrant' {}

Length of output: 33636


Script:

#!/bin/bash
# Description: Locate the definition of the `nonReentrant` modifier in Solidity files.

# Find all Solidity files and search for the definition of the `nonReentrant` modifier.
fd -e sol -x grep -H 'modifier nonReentrant' {}

Length of output: 19429


Script:

#!/bin/bash
# Description: Extract the implementation of the `nonReentrant` modifier from Solidity files.

# Extract the implementation of the `nonReentrant` modifier from `ZetaTokenConsumerPancakeV3.strategy.sol`.
fd -e sol -x grep -A 5 'modifier nonReentrant' ./contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol

Length of output: 172661

test/fuzz/ERC20CustodyNewEchidnaTest.sol (1)

20-20: Verify the appropriateness of the hardcoded address.

The hardcoded address 0x123 is added as an additional parameter to the initialize function of the GatewayEVM contract. Ensure that this address is appropriate for the intended functionality.

contracts/prototypes/evm/ZetaConnectorNative.sol (4)

1-10: LGTM! Imports and contract declaration are appropriate.

The imports and the use of SafeERC20 for safe token transfers are correctly implemented.


11-19: LGTM! Constructor and withdraw function are correctly implemented.

The constructor correctly initializes the base contract, and the withdraw function securely transfers tokens and emits an event.


21-30: LGTM! withdrawAndCall function is correctly implemented.

The function securely transfers tokens to the gateway and correctly forwards the call to the gateway contract.


32-36: LGTM! receiveTokens function is correctly implemented.

The function securely transfers tokens from the sender to the contract.

contracts/prototypes/evm/ERC20CustodyNew.sol (4)

Line range hint 1-10:
LGTM! Imports and contract declaration are appropriate.

The imports and the use of SafeERC20 for safe token transfers and ReentrancyGuard to prevent reentrancy attacks are correctly implemented.


Line range hint 11-19:
LGTM! Constructor and withdraw function are correctly implemented.

The constructor correctly initializes the gateway and checks for a zero address. The withdraw function securely transfers tokens and emits an event.


Line range hint 20-50:
LGTM! withdrawAndCall function is correctly implemented.

The function securely transfers tokens to the gateway and correctly forwards the call to the gateway contract.


51-64: LGTM! withdrawAndRevert function is correctly implemented.

The function securely transfers tokens to the gateway and correctly forwards the call to the gateway contract's revert function.

testFoundry/GatewayEVMUpgrade.t.sol (6)

11-11: LGTM!

The import statement for ZetaConnectorNonNative is correctly added.


16-17: LGTM!

The import statement for TestERC20 is correctly added.


28-30: LGTM!

The variable declarations for zetaConnector and zeta are correctly added.


41-41: LGTM!

The initialization of the zeta token is correctly added.


45-45: LGTM!

The ERC1967Proxy initialization call is correctly updated to include the zeta token.


50-54: LGTM!

The initialization of the zetaConnector and the setConnector method call are correctly added.

readme.md (1)

1-5: LGTM!

The new section highlighting the development of Version 2 (V2) of the smart contract architecture is correctly added and provides the necessary information.

test/prototypes/GatewayEVMUniswap.spec.ts (2)

59-64: LGTM!

The deployment process is correctly updated to include the ZetaConnectorNonNative contract and the TestERC20 token named "Zeta".


69-71: LGTM!

The method calls to setCustody and setConnector are correctly added to establish relationships between the gateway and the custody contract as well as the Zeta connector.

testFoundry/GatewayZEVM.t.sol (3)

12-12: Good practice: Specific import.

The change to import a more specific interface IGatewayZEVM.sol improves clarity and maintainability.


54-54: Improvement: Explicit event emission.

The change to use address(zrc20) in the Withdrawal event emission makes the event more explicit and clear.


66-66: Improvement: Explicit event emission.

The change to use address(zrc20) in the Withdrawal event emission makes the event more explicit and clear.

scripts/worker.ts (2)

134-134: Improvement: Enhanced event parameter clarity.

The change to the Withdrawal event signature, updating the second parameter from bytes to to address zrc20, enhances the clarity of the event's parameters.


138-138: LGTM! But verify the argument indexing.

The change to the index used to retrieve the message from the args array suggests a reordering of the arguments. Ensure that the new indexing is correct and does not break existing functionality.

contracts/prototypes/zevm/GatewayZEVM.sol (12)

7-13: Necessary imports for new functionality.

The new imports for ReentrancyGuardUpgradeable and IWZETA are necessary for the introduced functionality and security enhancements.


28-36: Enhanced robustness in initialization.

The update to the initialize function to accept an address parameter _zetaToken and include a validation check for a zero address enhances the robustness of the initialization process.


Line range hint 41-50:
Improved clarity in function naming and logic.

The renaming of _withdraw to _withdrawZRC20 and the logic updates for handling ZRC20 tokens make the function's purpose more explicit and improve code clarity.


56-61: Centralized ZETA transfer logic.

The new _transferZETA function centralizes the ZETA transfer logic, improving the contract's structure.


64-66: Enhanced security with nonReentrant modifier.

The update to the withdraw function to include the nonReentrant modifier enhances security against reentrancy attacks.


70-73: Enhanced security with nonReentrant modifier.

The update to the withdrawAndCall function to include the nonReentrant modifier enhances security against reentrancy attacks.


76-79: Enhanced security with nonReentrant modifier.

The update to the withdraw function for ZETA to include the nonReentrant modifier enhances security against reentrancy attacks.


82-84: Enhanced security with nonReentrant modifier.

The update to the withdrawAndCall function for ZETA to include the nonReentrant modifier enhances security against reentrancy attacks.


88-88: Enhanced security with nonReentrant modifier.

The update to the call function to include the nonReentrant modifier enhances security against reentrancy attacks.


141-152: Increased flexibility in cross-chain operations.

The new depositAndCall function provides users with more flexibility in cross-chain operations by allowing them to deposit ZETA and interact with target contracts.


157-167: Enhanced functionality with revert operations.

The new executeRevert function enhances the contract's functionality by providing a way to revert operations on the ZEVM.


172-183: Increased flexibility in cross-chain operations.

The new depositAndRevert function provides users with more flexibility in cross-chain operations by allowing them to deposit ZRC20 tokens and revert a contract on the ZEVM.

testFoundry/GatewayEVMZEVM.t.sol (6)

10-10: LGTM! Import statements are appropriate.

The new import statements for ZetaConnectorNonNative.sol and specific interfaces are necessary for the new functionality.

Also applies to: 21-23


34-36: LGTM! State variables are necessary for new functionality.

The new state variables zetaConnector and zeta are required for the new functionality introduced in the tests.


58-59: LGTM! Constructor changes are appropriate.

The addition of the zeta token initialization is necessary for the new functionality.


62-62: LGTM! Proxy initialization changes are appropriate.

The inclusion of the zeta token address in the proxy initialization for GatewayEVM is necessary for the new functionality.


69-69: LGTM! Method call changes are appropriate.

The setConnector method of gatewayEVM being called with the address of the zetaConnector is necessary for the new functionality.


150-150: LGTM! Event emission changes are appropriate.

The inclusion of the address of zrc20 in the Withdrawal event emission is necessary for the new functionality.

contracts/prototypes/evm/GatewayEVM.sol (11)

9-12: LGTM! Import statements are appropriate.

The new import statements for ReentrancyGuardUpgradeable.sol and ZetaConnectorNewBase.sol are necessary for the new functionality and security enhancements.


27-29: LGTM! State variables are necessary for new functionality.

The new state variables zetaConnector, zeta, and zetaToken are required for the new functionality and security enhancements.


36-46: LGTM! Initialize function changes are appropriate.

The initialize function now accepts an additional parameter _zetaToken and includes a zero address check, enhancing the contract's robustness against improper initialization.


58-66: LGTM! New executeRevert function is appropriate.

The executeRevert function enhances the contract's interaction capabilities with external contracts by handling revert operations.


Line range hint 88-107:
LGTM! ExecuteWithERC20 function changes are appropriate.

The executeWithERC20 function now includes reentrancy protection and approval checks, enhancing the security and functionality of the contract.


112-122: LGTM! RevertWithERC20 function changes are appropriate.

The revertWithERC20 function now includes reentrancy protection, enhancing the security of the contract.


135-139: LGTM! Deposit function changes are appropriate.

The deposit function now includes a zero amount check and emits an event, enhancing the functionality and traceability of the contract.


Line range hint 154-162:
LGTM! DepositAndCall function changes are appropriate.

The depositAndCall function now includes a zero amount check and emits an event, enhancing the functionality and traceability of the contract.


175-180: LGTM! SetConnector function changes are appropriate.

The setConnector function now includes a zero address check and an initialization check, enhancing the configurability and security of the contract.


186-197: LGTM! TransferFromToAssetHandler function is appropriate.

The transferFromToAssetHandler function centralizes the logic for transferring tokens, enhancing maintainability and reducing code duplication.


199-208: LGTM! TransferToAssetHandler function is appropriate.

The transferToAssetHandler function centralizes the logic for transferring tokens, enhancing maintainability and reducing code duplication.

contracts/prototypes/evm/GatewayEVMUpgradeTest.sol (8)

9-11: LGTM! Import statements are appropriate.

The new import statements for ReentrancyGuardUpgradeable.sol and ZetaConnectorNewBase.sol are necessary for the new functionality and security enhancements.


26-28: LGTM! State variables are necessary for new functionality.

The new state variables zetaConnector, zeta, and zetaToken are required for the new functionality and security enhancements.


37-47: LGTM! Initialize function changes are appropriate.

The initialize function now accepts an additional parameter _zetaToken and includes a zero address check, enhancing the contract's robustness against improper initialization.


61-67: LGTM! New executeRevert function is appropriate.

The executeRevert function enhances the contract's interaction capabilities with external contracts by handling revert operations.


89-108: LGTM! ExecuteWithERC20 function changes are appropriate.

The executeWithERC20 function now includes reentrancy protection and approval checks, enhancing the security and functionality of the contract.


112-124: LGTM! RevertWithERC20 function changes are appropriate.

The revertWithERC20 function now includes reentrancy protection, enhancing the security of the contract.


136-139: LGTM! Deposit function changes are appropriate.

The deposit function now includes a zero amount check and emits an event, enhancing the functionality and traceability of the contract.


156-162: LGTM! DepositAndCall function changes are appropriate.

The depositAndCall function now includes a zero amount check and emits an event, enhancing the functionality and traceability of the contract.

testFoundry/GatewayEVM.t.sol (4)

46-46: Ensure correct initialization of GatewayEVM.

Verify that the additional parameter address(zeta) in the GatewayEVM constructor call is correct and does not break existing functionality.


54-54: Ensure correct usage of setConnector method.

Verify that the setConnector method call with the address of zetaConnector is correct and does not break existing functionality.


202-202: Ensure correct initialization of GatewayEVM.

Verify that the additional parameter address(zeta) in the GatewayEVM constructor call is correct and does not break existing functionality.


209-209: Ensure correct usage of setConnector method.

Verify that the setConnector method call with the address of zetaConnector is correct and does not break existing functionality.

@@ -15,6 +15,7 @@ contract GatewayEVMEchidnaTest is GatewayEVM {

constructor() {
tssAddress = echidnaCaller;
zetaConnector = address(0x123);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider making the hardcoded address configurable.

The hardcoded address value 0x123 may be a placeholder. It is recommended to make this address configurable for better flexibility and maintainability.

-        zetaConnector = address(0x123);
+        zetaConnector = address(0x123); // Consider making this address configurable

Committable suggestion was skipped due to low confidence.

@@ -13,7 +13,23 @@
uint256 amount,
bytes calldata message
) external override {
string memory decodedMessage = abi.decode(message, (string));
string memory decodedMessage;

Check warning

Code scanning / Slither

Uninitialized local variables Medium

uint256 amount,
bytes calldata message
) external override {
string memory decodedMessage;

Check warning

Code scanning / Slither

Uninitialized local variables Medium

Comment on lines +184 to +195
function transferFromToAssetHandler(address from, address token, uint256 amount) private {
if (token == zetaToken) { // transfer to connector
// transfer amount to gateway
IERC20(token).safeTransferFrom(from, address(this), amount);
// approve connector to handle tokens depending on connector version (eg. lock or burn)
IERC20(token).approve(zetaConnector, amount);
// send tokens to connector
ZetaConnectorNewBase(zetaConnector).receiveTokens(amount);
} else { // transfer to custody
IERC20(token).safeTransferFrom(from, custody, amount);
}
}

Check warning

Code scanning / Slither

Unused return Medium

Comment on lines +197 to +206
function transferToAssetHandler(address token, uint256 amount) private {
if (token == zetaToken) { // transfer to connector
// approve connector to handle tokens depending on connector version (eg. lock or burn)
IERC20(token).approve(zetaConnector, amount);
// send tokens to connector
ZetaConnectorNewBase(zetaConnector).receiveTokens(amount);
} else { // transfer to custody
IERC20(token).safeTransfer(custody, amount);
}
}

Check warning

Code scanning / Slither

Unused return Medium

return result;
}

// Called by the TSS
// Calling onRevert directly
function executeRevert(address destination, bytes calldata data) public payable {

Check notice

Code scanning / Slither

Missing zero address validation Low

if (!success) revert ExecutionFailed();

return result;
}

// Called by the TSS
// Calling onRevert directly
function executeRevert(address destination, bytes calldata data) public payable {

Check notice

Code scanning / Slither

Missing zero address validation Low

Comment on lines +58 to +64
function executeRevert(address destination, bytes calldata data) public payable {
(bool success, bytes memory result) = destination.call{value: msg.value}("");
if (!success) revert ExecutionFailed();
Revertable(destination).onRevert(data);

emit Reverted(destination, msg.value, data);
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

Comment on lines +60 to +66
function executeRevert(address destination, bytes calldata data) public payable {
(bool success, bytes memory result) = destination.call{value: msg.value}("");
if (!success) revert ExecutionFailed();
Revertable(destination).onRevert(data);

emit Reverted(destination, msg.value, data);
}

Check notice

Code scanning / Slither

Reentrancy vulnerabilities Low

Comment on lines +60 to +66
function executeRevert(address destination, bytes calldata data) public payable {
(bool success, bytes memory result) = destination.call{value: msg.value}("");
if (!success) revert ExecutionFailed();
Revertable(destination).onRevert(data);

emit Reverted(destination, msg.value, data);
}

Check warning

Code scanning / Slither

Low-level calls Warning

Comment on lines +58 to +64
function executeRevert(address destination, bytes calldata data) public payable {
(bool success, bytes memory result) = destination.call{value: msg.value}("");
if (!success) revert ExecutionFailed();
Revertable(destination).onRevert(data);

emit Reverted(destination, msg.value, data);
}

Check warning

Code scanning / Slither

Low-level calls Warning

@skosito skosito merged commit b921999 into main Jul 19, 2024
11 checks passed
@skosito skosito deleted the revert-specs branch July 19, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement new revert specs
4 participants