blockchainguyy commented 2 months ago

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.

"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": 1
    },

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:

npx hardhat test --network flow --grep AxelarGateway

Comparison

When the same commands are executed for other EVM-compatible chains, these tests should run smoothly. To verify this, use the following command:

npx hardhat test --network fantom --grep AxelarGateway

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:

{
  "jsonrpc": "2.0",
  "id": 117,
  "error": { "code": -32000, "message": "internal error" }
}

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.

m-Peter commented 2 months ago

@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.

m-Peter commented 2 months ago

@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 selfdestruct ?

Update:

blockchainguyy commented 2 months ago

Thanks @m-Peter ! Were you able to run test on Line 1077?

This is a different error on my end where the expected event is not emitted. I can still reproduce it on my end.

Sample tx: https://eth.flowscan.io/tx/0xf49c5e3fe972f9428ee366b41ca6603845fb9da8fb33ead462e4a35334f22c63?tab=logs

m-Peter commented 2 months ago

@blockchainguyy Yes, I was also able to reproduce this failure locally. I will prepare a local build with the fix in https://github.com/onflow/flow-go/pull/6289, to see if this test is also related to the selfdestruct state error. And I also want to see if it will fix the other 3 errors.

m-Peter commented 2 months ago

@blockchainguyy I ran again the tests with the fix in https://github.com/onflow/flow-go/pull/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.

m-Peter commented 2 months ago

@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 selfdestruct is missing this particular case. Can you elaborate on what is the test doing, on a high-level?

blockchainguyy commented 2 months ago

@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:

Does having balance change your [isContract](https://github.com/axelarnetwork/axelar-cgp-solidity/blob/be9f5c29b1ca4db788f59ff31c667c502d76211a/contracts/AxelarGateway.sol#L613)() implementation?
Having ether in the contract removes events from the mined transaction

Note: you should also check ethers are transferred to the gateway when it selfdestructs.

m-Peter commented 2 months ago

@blockchainguyy After some digging, I found out that in this particular case, the burnToken function returns early on this line. Which is exactly what you mentioned above, on the first bullet :sweat_smile: I'll need to investigate further :pray:

blockchainguyy commented 2 months ago

@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.

m-Peter commented 2 months ago

@blockchainguyy To try something out, I by-passed the if (depositHandlerAddress.isContract()) return; condition, but then the transaction reverts when coming across this line:

DepositHandler depositHandler = new DepositHandler{ salt: salt }();

Why could that be?

m-Peter commented 1 month ago

@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 depositHandlerAddress already has received a transfer. When the above address has not received any transfer, we get:

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 StorageRoot & ContractHash is:

// https://eips.ethereum.org/EIPS/eip-1052
keccak256('') == 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470

cc @ramtinms

ramtinms commented 1 month ago

I see, let me try to reproduce

ramtinms commented 1 month ago

This should fix the issue https://github.com/onflow/flow-go/pull/6295

m-Peter commented 1 month ago

@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 has 1 failure. And RpcCompatibility seems like a good candidate to try out as well.

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

blockchainguyy commented 1 month ago

@m-Peter can you supply gas option like this in your local version axelar-chains-config/info/testnet.json:

    "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 gasLimit, I am able to run these tests without hiccups:

  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).

m-Peter commented 1 month ago

@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 eth_estimateGas, we add some buffer gas, in order to make sure that the return value is sufficient for a transaction to execute successfully. This is only for certain edge cases. See: https://github.com/onflow/flow-go/blob/master/fvm/evm/emulator/emulator.go#L291-L308. I did check the values, and the gas used is indeed 27_613, which is less than 30_000.

I am running these tests on a local JSON-RPC server, to make sure that it works with the latest changes. I am still getting these failures:

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 should upgrade AxelarGateway through InterchainGovernance proposal ?

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 selfdestruct a contract in a future transaction.

blockchainguyy commented 1 month ago

Regarding the gas calculation, Flow EVM has the same gas calculation like normal EVM, it is just that for eth_estimateGas, we add some buffer gas, in order to make sure that the return value is sufficient for a transaction to execute successfully. This is only for certain edge cases. See: https://github.com/onflow/flow-go/blob/master/fvm/evm/emulator/emulator.go#L291-L308.

eth_estimateGas should try to provide actual estimates, because buffers are already added by providers broadcasting the transaction, and it can block users with limited balance to interact with your nodes even if they have enough balance to transact. Anyway, not a blocker for us to integrate so it can be handled separately.

No need to run DepositHandler tests for comaptibility, our new amplifier gateway (which will be deployed on Flow) has removed selfdestruct so it won't be needed.

For AxelarGatewayUpgrade pls set gasOptions in you testnet config as done here: https://github.com/axelarnetwork/axelar-contract-deployments/blob/218b366103b2f1fc9c0a9ad602758725b50b5392/axelar-chains-config/info/testnet.json#L114

I have shared my config setting in this comment already: https://github.com/onflow/flow-evm-gateway/issues/404#issuecomment-2271156692

estimateGas is not able to calculate gas for the required upgrade properly and hence we need to specify the limit explicitly.

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.

blockchainguyy commented 1 month ago

@m-Peter ^

m-Peter commented 1 month ago

@blockchainguyy The 2 PRs which fix the test failures for AxelarGateway test file are merged:

I did ran the AxelarGateway test suite locally, with a custom build containing the above fixes. The result is:

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 AxelarGateway test suite.

And the same goes for RpcCompatibility:

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 eth_estimateGas test case:

expect(gas).to.be.lt(30500);

m-Peter commented 1 month ago

eth_estimateGas should try to provide actual estimates, because buffers are already added by providers broadcasting the transaction, and it can block users with limited balance to interact with your nodes even if they have enough balance to transact. Anyway, not a blocker for us to integrate so it can be handled separately.

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. Flow EVM, on the other hand, will do a dry run of the transaction without committing the state. This will return the actual gas used from the transaction. However, in order to run the transaction, the gas limit has to be higher in many cases, although it won't be consumed. This is for cases such as refunds for slot clearance, prevention of re-entrancy attacks, and create / create2 opcodes.

No need to run DepositHandler tests for comaptibility, our new amplifier gateway (which will be deployed on Flow) has removed selfdestruct so it won't be needed.

Sounds good :+1:

For AxelarGatewayUpgrade pls set gasOptions in you testnet config as done here: https://github.com/axelarnetwork/axelar-contract-deployments/blob/218b366103b2f1fc9c0a9ad602758725b50b5392/axelar-chains-config/info/testnet.json#L114

I have shared my config setting in this comment already: #404 (comment)

estimateGas is not able to calculate gas for the required upgrade properly and hence we need to specify the limit explicitly.

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.

blockchainguyy commented 1 month ago

Then we just have to be sure the merge does not break things. :) Thanks @m-Peter! 🙌🏼

m-Peter commented 1 month ago

@blockchainguyy Thanks for the guidance on running these tests :bow: Let's hope things will run smoothly on PreviewNet, after we deploy those fixes :pray:

m-Peter commented 1 month ago

Then we just have to be sure the merge does not break things. :) Thanks @m-Peter! 🙌🏼

@blockchainguyy I was finally able to find out why this AxelarGatewayUpgrade test fails locally :pray: This is the result of eth_getTransactionReceipt:

{
  "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 revertReason, we see that the revert is coming from this line: https://github.com/axelarnetwork/axelar-gmp-sdk-solidity/blob/main/contracts/utils/TimeLock.sol#L76. This is because, when running the JSON-RPC locally against the Flow Emulator, there are basically no delays between blocks, it runs very fast. That's why the test succeeds on a live network, such as PreviewNet, but fails locally. So after this, I think we are on a good state, and we're just waiting for a deployment on the necessary changes on PreviewNet :pray: .

m-Peter commented 1 month ago

@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 AxelarGateway test file, are now passing :tada:

AxelarGatewayUpgrade
    ✔ should deploy gateway with the correct modules (2495ms)
    ✔ should upgrade AxelarGateway through InterchainGovernance proposal (83557ms)

2 passing (3m)

Both tests on AxelarGatewayUpgrade test file, still pass :tada:

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 RpcCompatibility test file, are now passing :tada: Note: For this I made the following change locally:

// 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 eth_estimateGas. We will re-work this, in the coming days. But it should be good for now.

Let me know if everything's looking good, so we can close the issue :pray:

onflow / flow-evm-gateway