-
Notifications
You must be signed in to change notification settings - Fork 10
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
Axelar Gateway compatibility issues #404
Comments
@blockchainguyy Thanks for the detailed issue. I will follow your steps and run these commands locally, to see what could be causing the compatibility failures. |
@blockchainguyy I was able to re-produce the failures locally, all the following tests: fail with: state error: invalid operation, can't selfdestruct an account that is just created Can you point me to the function that is calling Update: |
@blockchainguyy Yes, I was also able to reproduce this failure locally. I will prepare a local build with the fix in onflow/flow-go#6289, to see if this test is also related to the |
@blockchainguyy I ran again the tests with the fix in onflow/flow-go#6289, and got: AxelarGateway
command burnToken
burn token positive tests
burnToken external gas: 355039
✔ should allow the operators to burn external tokens (786ms)
AxelarGateway
command burnToken
burn token positive tests
✔ should allow the operators to burn the external token multiple times from the same address (881ms)
AxelarGateway
external tokens
✔ should support external ERC20 token (928ms) So we have 3/4 failing tests fixed. I still have to look into Line 1077. |
@blockchainguyy So the test beginning at Line 1077 still fails, and If I comment this line it passes. I am not sure what is happening here, but I guess something to do with the fact that the deposit address already has some balance and |
@m-Peter This test sends some ethers (native token of the chain), as well as an ERC20 token on a destination contract before it is created. Then when the execute function is called, the destination contract is deployed and self destructs in the same transaction. Before it self destructs, it sends the ERC20 tokens as well as Ether to the Gateway contract. Some guesses on what could be wrong:
Note: you should also check ethers are transferred to the gateway when it selfdestructs. |
@blockchainguyy After some digging, I found out that in this particular case, the |
@m-Peter this is the function which is failing: https://github.com/axelarnetwork/axelar-gmp-sdk-solidity/blob/3157ba4f704de408e356ad8236b5d81bbc82d7cb/contracts/libs/ContractAddress.sol#L6 Maybe you are storing some data on the address if you send native tokens to the contract which would be different from other EVMs. |
@blockchainguyy To try something out, I by-passed the DepositHandler depositHandler = new DepositHandler{ salt: salt }(); Why could that be? |
@blockchainguyy I have found the root cause of this test failure, it is basically a contract address collision, when it encounters this line: DepositHandler depositHandler = new DepositHandler{ salt: salt }(); given that the Nonce: 0
StorageRoot: 0x0000000000000000000000000000000000000000000000000000000000000000
ContractHash: 0x0000000000000000000000000000000000000000000000000000000000000000 but when it has received a transfer, we get: Nonce: 0
StorageRoot: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
ContractHash: 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 The value contained in // https://eips.ethereum.org/EIPS/eip-1052
keccak256('') == 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 cc @ramtinms |
I see, let me try to reproduce |
This should fix the issue onflow/flow-go#6295 |
@blockchainguyy The fix linked above does indeed fix the issue for the last failing test: AxelarGateway
command burnToken
burn token positive tests
burnToken external gas: 355059
✔ should allow the operators to burn external tokens (617ms)
✔ should allow the operators to burn external tokens even if the deposit address has ether (738ms)
✔ should allow the operators to burn the external token multiple times from the same address (626ms)
external tokens
✔ should support external ERC20 token (709ms)
4 passing (5s) Are there any other test suites that I should try out? For example, I noticed that AxelarGatewayUpgrade
✔ should deploy gateway with the correct modules (137ms)
1) should upgrade AxelarGateway through InterchainGovernance proposal
1 passing (14s)
1 failing 1) RpcCompatibility
eth_estimateGas
should support RPC method eth_estimateGas like ethereum mainnet:
AssertionError: expected 47711 to be below 30000. The numerical values of the given "ethers.BigNumber" and "number" inputs were compared, and they differed.
+ expected - actual
-47711
+30000 1) DepositHandler
destroy
should destroy the contract and send ETH to destination:
AssertionError: expected '0x608060405234801561001057600080fd5b5…' to equal '0x'
+ expected - actual
-0x608060405234801561001057600080fd5b50600436106100355760003560e01c8062f55d9d1461003a5780631cff79cd1461004f575b600080fd5b61004d6100483660046101da565b610079565b005b61006261005d3660046101fc565b6100bb565b60405161007092919061027f565b60405180910390f35b6002600054141561009d5760405163caa30f5560e01b815260040160405180910390fd5b600260005573ffffffffffffffffffffffffffffffffffffffff8116ff5b60006060600260005414156100e35760405163caa30f5560e01b815260040160405180910390fd5b600260005573ffffffffffffffffffffffffffffffffffffffff85163b610136576040517f6f7c43f100000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b8473ffffffffffffffffffffffffffffffffffffffff16848460405161015d9291906102de565b6000604051808303816000865af19150503d806000811461019a576040519150601f19603f3d011682016040523d82523d6000602084013e61019f565b606091505b50600160005590969095509350505050565b803573ffffffffffffffffffffffffffffffffffffffff811681146101d557600080fd5b919050565b6000602082840312156101ec57600080fd5b6101f5826101b1565b9392505050565b60008060006040848603121561021157600080fd5b61021a846101b1565b9250602084013567ffffffffffffffff8082111561023757600080fd5b818601915086601f83011261024b57600080fd5b81358181111561025a57600080fd5b87602082850101111561026c57600080fd5b6020830194508093505050509250925092565b821515815260006020604081840152835180604085015260005b818110156102b557858101830151858201606001528201610299565b818111156102c7576000606083870101525b50601f01601f191692909201606001949350505050565b818382376000910190815291905056fea264697066735822122032cb5e746816b7fac95205c068b30da37bd40119a57265be331c162cae74712464736f6c63430008090033
+0x |
@m-Peter can you supply gas option like this in your local version "flow": {
"name": "flow",
"id": "flow",
"axelarId": "flow",
"chainId": 646,
"rpc": "https://previewnet.evm.nodes.onflow.org",
"tokenSymbol": "FLOW",
"contracts": {
"AxelarGateway": {},
"Multisig": {
"threshold": 2,
"signers": [
"0x15837c1318AB83d99b19392Fd4811813f520d843",
"0x64247a441CeF0b7A46614AC34d046c0fdfe35954",
"0xEE64c8eb48437DbD2D5B8598dc4A3E8a6c8CEaD9"
]
}
},
"explorer": {
"name": "Flowscan",
"url": "http://eth.flowscan.io",
"api": ""
},
"gasOptions": {
"gasLimit": 10000000
},
"confirmations": 0
} once I specify AxelarGatewayUpgrade
✔ should deploy gateway with the correct modules (4598ms)
✔ should upgrade AxelarGateway through InterchainGovernance proposal (70704ms)
2 passing (3m) You can ignore the failing RPC, its just to flag on our end if a chain's gas calculation is different from normal EVM, our SDK team should be able to handle it using your docs (if it is properly documented). |
@blockchainguyy I am continuing the investigation of those failures. Regarding the gas calculation, Flow EVM has the same gas calculation like normal EVM, it is just that for I am running these tests on a local JSON-RPC server, to make sure that it works with the latest changes. AxelarGatewayUpgrade
✔ should deploy gateway with the correct modules (137ms)
1) should upgrade AxelarGateway through InterchainGovernance proposal
1 passing (14s)
1 failing Do I need any special setup for 1) DepositHandler
destroy
should destroy the contract and send ETH to destination:
AssertionError: expected '0x608060405234801561001057600080fd5b5…' to equal '0x'
+ expected - actual
-0x608060405234801561001057600080fd5b50600436106100355760003560e01c8062f55d9d1461003a5780631cff79cd1461004f575b600080fd5b61004d6100483660046101da565b610079565b005b61006261005d3660046101fc565b6100bb565b60405161007092919061027f565b60405180910390f35b6002600054141561009d5760405163caa30f5560e01b815260040160405180910390fd5b600260005573ffffffffffffffffffffffffffffffffffffffff8116ff5b60006060600260005414156100e35760405163caa30f5560e01b815260040160405180910390fd5b600260005573ffffffffffffffffffffffffffffffffffffffff85163b610136576040517f6f7c43f100000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b8473ffffffffffffffffffffffffffffffffffffffff16848460405161015d9291906102de565b6000604051808303816000865af19150503d806000811461019a576040519150601f19603f3d011682016040523d82523d6000602084013e61019f565b606091505b50600160005590969095509350505050565b803573ffffffffffffffffffffffffffffffffffffffff811681146101d557600080fd5b919050565b6000602082840312156101ec57600080fd5b6101f5826101b1565b9392505050565b60008060006040848603121561021157600080fd5b61021a846101b1565b9250602084013567ffffffffffffffff8082111561023757600080fd5b818601915086601f83011261024b57600080fd5b81358181111561025a57600080fd5b87602082850101111561026c57600080fd5b6020830194508093505050509250925092565b821515815260006020604081840152835180604085015260005b818110156102b557858101830151858201606001528201610299565b818111156102c7576000606083870101525b50601f01601f191692909201606001949350505050565b818382376000910190815291905056fea264697066735822122032cb5e746816b7fac95205c068b30da37bd40119a57265be331c162cae74712464736f6c63430008090033
+0x With the addition of https://eips.ethereum.org/EIPS/eip-6780, does this test make any sense now? It shouldn't be possible to |
No need to run For I have shared my config setting in this comment already: #404 (comment)
There is a sense of urgency on our end to proceed with integration, so if things could be merged and deployed quickly it would be great. |
@m-Peter ^ |
@blockchainguyy The 2 PRs which fix the test failures for
I did ran the AxelarGateway
axelar gateway proxy
✔ should revert on invalid gateway implementation address (52ms)
✔ should revert if gateway setup fails (489ms)
✔ should fail on receiving native value (769ms)
constructor checks
✔ should revert if auth module is not a contract (54ms)
✔ should revert if token deployer is not a contract (66ms)
✔ check internal constants (255ms)
deployment params
✔ should get the correct governance address
✔ should get the correct mint limiter address
✔ should get the correct auth module
✔ auth module should have the correct owner (43ms)
✔ should get the correct token deployer
check external methods that should only be called by self
✔ should fail on external call to deployToken
✔ should fail on external call to mintToken
✔ should fail on external call to burnToken (41ms)
✔ should fail on external call to approveContractCall
✔ should fail on external call to approveContractCallWithMint
✔ should fail on external call to transferOperatorship (41ms)
should preserve the bytecode [ @skip-on-coverage ]
✔ should preserve the same proxy bytecode for each EVM
✔ should preserve the implementation bytecode for each EVM
✔ should have the same deposit handler bytecode preserved for each EVM
✔ should have the same token bytecode preserved for each EVM
setTokenMintLimits
✔ should allow governance to set a token's daily limit (256ms)
gateway operators
✔ should allow transferring governance (235ms)
✔ should allow transferring mint limiter (335ms)
upgrade
✔ should allow governance to upgrade to the correct implementation (488ms)
✔ should allow governance to upgrade to the correct implementation with new governance and mint limiter (441ms)
✔ should allow governance to upgrade to the correct implementation with new operators (4480ms)
✔ should allow governance to upgrade to the correct implementation with new governance and operators (459ms)
✔ should allow governance to upgrade to the same implementation with new governance (230ms)
✔ should not allow governance to upgrade to a wrong implementation (586ms)
✔ should not allow calling the setup function directly (213ms)
✔ should not allow malicious proxy to call setup function directly and transfer governance or mint limiter (236ms)
✔ should not allow calling the upgrade on the implementation (337ms)
✔ should revert on upgrade if setup fails for any reason (321ms)
chain id
✔ should fail if chain id mismatches
command deployToken
deployToken gas: 1318975
✔ should allow operators to deploy a new token (253ms)
✔ should not deploy a duplicate token (278ms)
command mintToken
✔ should not allow the operators to mint tokens exceeding the daily limit (434ms)
✔ should allow the operators to mint tokens (166ms)
✔ should not mint wrong symbols (160ms)
command burnToken
burn token positive tests
burnToken internal gas: 111515
✔ should allow the operators to burn internal tokens (707ms)
burnToken external gas: 355051
✔ should allow the operators to burn external tokens (678ms)
✔ should allow the operators to burn external tokens even if the deposit address has ether (859ms)
✔ should allow the operators to burn the external token multiple times from the same address (749ms)
burn token negative tests
✔ should fail if symbol does not correspond to internal token (293ms)
✔ should fail to burn external tokens if deposit handler execute reverts (119ms)
✔ should fail to burn external tokens if deposit handler execute fails (305ms)
command transferOperatorship
transferOperatorship gas: 152993
✔ should allow operators to transfer operatorship (142ms)
✔ should not allow transferring operatorship to address zero (144ms)
✔ should allow the previous operators to mint and burn token (1165ms)
✔ should not allow the previous operators to transfer operatorship (250ms)
✔ should not allow operatorship transfer to the previous operators (256ms)
✔ should not allow multiple operatorship transfers in one batch (127ms)
sendToken
send token negative tests
✔ should fail if token deployment fails (164ms)
send token positive tests
sendToken internal gas: 60518
✔ should burn internal token and emit an event (492ms)
sendNative external gas: 76125
✔ should lock external token and emit an event (571ms)
external tokens
✔ should fail if external ERC20 token address is invalid (149ms)
✔ should support external ERC20 token (763ms)
batch commands
✔ should revert on mismatch between commandID and command/params length (89ms)
✔ should batch execute multiple commands and skip any unknown commands (158ms)
✔ should not execute the same commandID twice (157ms)
callContract
callContract gas: 34582
✔ should emit an event (137ms)
callContractWithToken
✔ should revert if token does not exist
✔ should revert if token amount is invalid (39ms)
callContractWithToken internal gas: 63567
✔ should burn internal token and emit an event (504ms)
callContractWithToken external gas: 79223
✔ should lock external token and emit an event (596ms)
external contract approval and execution
✔ should approve and validate contract call (527ms)
✔ should approve and validate contract call with token (800ms)
deprecated functions
✔ should return correct value for allTokensFrozen
✔ should return correct value for tokenFrozen (55ms)
70 passing (1m) So all is good regarding the And the same goes for RpcCompatibility
✔ should support RPC method eth_blockNumber
✔ should support RPC method eth_call (188ms)
✔ should support RPC method eth_getCode
✔ should support RPC method eth_gasPrice
✔ should support RPC method eth_chainId
✔ should support RPC method eth_getTransactionCount (206ms)
✔ should support RPC method eth_sendRawTransaction [ @skip-on-coverage ] (164ms)
✔ should support RPC method eth_getBalance
✔ should support RPC method eth_syncing
✔ should support RPC method eth_subscribe (5164ms)
✔ should return consistent logIndex values between eth_getLogs and eth_getTransactionReceipt (164ms)
eth_getLogs
✔ should support RPC method eth_getLogs
Achieved safety for the block instantly
✔ supports safe tag
Achieved finality for the block instantly
✔ supports finalized tag
rpc get transaction and blockByHash methods
✔ should support RPC method eth_getTransactionReceipt
✔ should support RPC method eth_getTransactionByHash
✔ should support RPC method eth_getBlockByHash
eth_getBlockByNumber
✔ should support RPC method eth_getBlockByNumber
✔ supports safe tag
✔ supports finalized tag
eth_estimateGas
✔ should support RPC method eth_estimateGas like ethereum mainnet
✔ should send tx with estimated gas (226ms)
eip-1559 supported rpc methods
✔ should support RPC method eth_maxPriorityFeePerGas (162ms)
✔ should send transaction based on RPC method eth_feeHistory pricing (149ms)
24 passing (7s) I have made a minor change for the expect(gas).to.be.lt(30500); |
Let me explain how gas estimation works for Flow EVM compared to normal Ethereum. Ethereum will repeatedly run a transaction, based on the given transaction arguments, with different gas limits until it finds the lowest gas limit that allows the transaction to run.
Sounds good 👍
I still can't get this to run locally, but I am able to run it against PreviewNet. So maybe something is wrong with my local setup. But as long as you can run it against the live network, then it should be good. |
Then we just have to be sure the merge does not break things. :) Thanks @m-Peter! 🙌🏼 |
@blockchainguyy Thanks for the guidance on running these tests 🙇 |
@blockchainguyy I was finally able to find out why this {
"jsonrpc": "2.0",
"id": 8,
"result": {
"blockHash": "0xbf31c9934df3d3e707b7a2d080ba36fb681f8d82fa777563014e44dbea0a0fe7",
"blockNumber": "0xd",
"contractAddress": null,
"cumulativeGasUsed": "0x6a33",
"effectiveGasPrice": "0xa",
"from": "0xa39d2A89E711802BF60b5A1a4E25a0e37958fEd3",
"gasUsed": "0x6a33",
"logs": [],
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"revertReason": "0x4019a5d3",
"status": "0x0",
"to": "0x747C95a3790b1ed6dB935b8AdE66a9C18De50dD0",
"transactionHash": "0x28a6e867dc3ad8426a0714cff8b2ebee408a44fafa39572189c3860e91c147db",
"transactionIndex": "0x0",
"type": "0x0"
}
} From the |
@blockchainguyy The fixes listed above have been deployed on PreviewNet, here are the results of running the Axelar Gateway testing suites: AxelarGateway
command burnToken
burn token positive tests
burnToken external gas: 355051
✔ should allow the operators to burn external tokens (69023ms)
✔ should allow the operators to burn external tokens even if the deposit address has ether (88080ms)
✔ should allow the operators to burn the external token multiple times from the same address (70223ms)
external tokens
✔ should support external ERC20 token (84484ms)
4 passing (9m) All 4 failures on AxelarGatewayUpgrade
✔ should deploy gateway with the correct modules (2495ms)
✔ should upgrade AxelarGateway through InterchainGovernance proposal (83557ms)
2 passing (3m) Both tests on RpcCompatibility
✔ should support RPC method eth_blockNumber (291ms)
✔ should support RPC method eth_call (17781ms)
✔ should support RPC method eth_getCode (237ms)
✔ should support RPC method eth_gasPrice (239ms)
✔ should support RPC method eth_chainId (205ms)
✔ should support RPC method eth_getTransactionCount (17427ms)
✔ should support RPC method eth_sendRawTransaction [ @skip-on-coverage ] (16550ms)
✔ should support RPC method eth_getBalance (223ms)
✔ should support RPC method eth_syncing (246ms)
✔ should support RPC method eth_subscribe (21696ms)
✔ should return consistent logIndex values between eth_getLogs and eth_getTransactionReceipt (16736ms)
eth_getLogs
✔ should support RPC method eth_getLogs (409ms)
Achieved safety for the block instantly
✔ supports safe tag (407ms)
Achieved finality for the block instantly
✔ supports finalized tag (409ms)
rpc get transaction and blockByHash methods
✔ should support RPC method eth_getTransactionReceipt (208ms)
✔ should support RPC method eth_getTransactionByHash (202ms)
✔ should support RPC method eth_getBlockByHash (396ms)
eth_getBlockByNumber
✔ should support RPC method eth_getBlockByNumber (1241ms)
✔ supports safe tag (401ms)
✔ supports finalized tag (380ms)
eth_estimateGas
✔ should support RPC method eth_estimateGas like ethereum mainnet (230ms)
✔ should send tx with estimated gas (17832ms)
eip-1559 supported rpc methods
✔ should support RPC method eth_maxPriorityFeePerGas (16790ms)
✔ should send transaction based on RPC method eth_feeHistory pricing (16283ms)
24 passing (3m) All tests on // before
expect(gas).to.be.lt(30000);
// after
expect(gas).to.be.lt(30500); This is due to the buffer gas which I described that we make use of in Let me know if everything's looking good, so we can close the issue 🙏 |
Axelar Gateway compatibility issues
This issue details the failing Gateway compatibility tests for Axelar.
Setup
Please follow the instructions in this link to set up your environment for live network testing. Additionally, you can use the provided configuration as a reference.
Replication
To run only the specified tests, add the only keyword to the following AxelarGateway tests:
Line 1024
Line 1077
Line 1135
Line 1694
Then run the following command:
Comparison
When the same commands are executed for other EVM-compatible chains, these tests should run smoothly. To verify this, use the following command:
TL;DR
When running the AxelarGateway compatibility tests, some RPC calls fail for certain transactions without providing a clear error message. The response typically looks like this:
This issue can be observed at this line.
Additionally, when running the Gateway tests, the gateway transaction does not produce the Transfer event as expected, unlike other chains. This can be tested at this line.
The text was updated successfully, but these errors were encountered: