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

Gravity bridge integration to a custom chain considerations #345

Open
iTiky opened this issue May 12, 2021 · 2 comments
Open

Gravity bridge integration to a custom chain considerations #345

iTiky opened this issue May 12, 2021 · 2 comments

Comments

@iTiky
Copy link

iTiky commented May 12, 2021

I've tried to integrate the Gravity bridge into our Cosmos-based chain and did some Ganache-cli local testing.
Overall it worked, but I've faced some issues I'd like to share.

  1. Cosmos Gas limit for Tx

    By default, it is set to 500_000_000u64.
    That is a huge value (we have a default limit of 1M).
    Cosmos errors don't tell you much why the Tx didn't pass through (the only way is to enable debug logs on the Cosmos side), so it took some time to debug.
    I think it would be a good idea to make it configurable on the orchestrator side.

  2. Ganache crashes and VM errors

    All the problems I've faced were about missing / huge Gas limits for Txs to the Ethereum node.

    For example:

    • deploy_erc20 (client)
      • I had to add Gas price, limit and multiplier to the call;
    • estimate_valset_cost, estimate_tx_batch_cost
      • I had to replace the let gas_limit = min((u64::MAX - 1).into(), our_balance.clone()); line to let gas_limit: Uint256 = 100000u64.into();;
      • I didn't dig into the problem, but I assume it is an int64 overflow when Ganache tries to multiply the Gas limit to Gas price;

    I'm not an Ethereum expert and those problems might be only Ganache related, but I thought it might be usefull to share.

  3. Cosmos gRPC protobuf

    Facing gRPC transport errors (orchestrator <-> Cosmos) I had to rebuild and replace the gravity.v1.rs using the orchestrator/proto_build script.
    That one is a bit strange as my quick look through the diff didn't show much of a difference, but it worked.

  4. Custom CosmosERC20 contract deployer

    Two problems here with an existing deployer (Gravity.sol contract and orchestrator/client):

    • _mint(_gravityAddress, MAX_UINT) mints tokens to the Gravity contract address by default, I've changed the constructor argument to msg.Sender;
    • An additional IncreaseAllowance call have to be made to enable the Eth -> Cosmos transfer for a particular Eth account;
  5. Orchestrator startup crash

    As I see, it's been already fixed: not initialized contact variable within the create_rpc_connections func.

  6. Custom Cosmos bech32 prefix

    We're using a custom prefix (not cosmos) for the account address prefix.
    That was fixed a few days ago too.

I hope this report might be helpful. I'd be happy to share more on my future integration attempts if needed.

@jtremback
Copy link
Contributor

Great report! We will look into these items.

@jkilpatr
Copy link
Contributor

Definitely agree that the orchestrator should be a lot more configurable.

Also we need to eliminate gas for oracle and signature submissions, the reason the gas amount needs to be so large is to optimize for a validator catching up on signatures or relaying thousands of deposit events at a time

Yes that problem is ganace specific, geth prints a warning though and everyone is always annoyed by it. I should be more specific about gas when doing simulated calls.

For the contract deployer, yes the debug erc20's it deploys send some to the gravity contract address. It's useful for tests not so useful in any other scenario.

Thanks for testing and providing feedback, I'll work on getting the problems not already resolved here fixed! Please let us know if you have any other problems or questions. I can be found on https://discordapp.com/invite/vw8twzR as well as the Cosmos discords under @jkilpatr

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

No branches or pull requests

3 participants