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

ICS29 Fee Middleware Support #1981

Merged
merged 166 commits into from
Oct 24, 2022
Merged

ICS29 Fee Middleware Support #1981

merged 166 commits into from
Oct 24, 2022

Conversation

soareschen
Copy link
Contributor

@soareschen soareschen commented Mar 21, 2022

Description

This is a work in progress to support ICS29 fee middleware. This branch tracks the changes made in cosmos/ibc-go#276, which is not yet merged.

Tasks:


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

Introduce TxConfig

Add test constructor for TxConfig

Add send_tx method on ChainDriver

Accept Wallet instead of WalletAddress for transferring tokens

Implement ibc_token_transfer

Simplify ibc_token_transfer

Token transfer using ibc_token_transfer is now working
@soareschen soareschen force-pushed the soares/ics29-fee-middleware branch from 655aca9 to 9bbc35a Compare May 11, 2022 10:58
@soareschen soareschen force-pushed the soares/ics29-fee-middleware branch from 099d9fa to bb15577 Compare May 11, 2022 13:06
@adizere
Copy link
Member

adizere commented Oct 13, 2022

@ljoss17 I just found a detailed tutorial by the IBC-go team on fee-enabled ft-transfers. Hope it proves useful in reviewing or understanding ICS29!

https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers

@ljoss17
Copy link
Contributor

ljoss17 commented Oct 13, 2022

@ljoss17 I just found a detailed tutorial by the IBC-go team on fee-enabled ft-transfers. Hope it proves useful in reviewing or understanding ICS29!

https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers

Awesome, thank you! I'll try it out!

Copy link
Contributor

@ljoss17 ljoss17 left a comment

Choose a reason for hiding this comment

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

Except for the two small suggestions, everything seems great.
The Fee Middleware was successfully tested the two following ways:

Using test_setup_with_fee_enabled_binary_channel.rs

Same method as the one used in this PR: #2682

Using chains spawned with simd binary

Inspired by this tutorial: https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers

  1. In the section https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers#setup use this forked repository, which has been updated in order to run with Hermes v1.0.0: https://github.com/ljoss17/simd-scripts
  2. If required, in the file network/hermes/variables.sh correctly configure HERMES_BINARY to point to the Hermes binary which has the ICS29 Fee Middleware Support
  3. In the section https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers#create-a-fee-enabled-ics20-fungible-token-transfer-channel instead of the command: hermes -c ./network/hermes/config.toml create channel test-1 connection-0 \ --port-a transfer \ --port-b transfer \ -v "{\"fee_version\":\"ics29-1\",\"app_version\":\"ics20-1\"}", use the command: hermes --config ./network/hermes/config.toml create channel --a-chain test-1 --a-connection connection-0 --a-port transfer --b-port transfer --channel-version "{\"fee_version\":\"ics29-1\",\"app_version\":\"ics20-1\"}"
  4. In the section https://github.com/cosmos/ibc-go/wiki/Fee-enabled-fungible-token-transfers#register-the-counterparty-payee, instead of using simd tx ibc-fee register-counterparty-payee to register the counterparty-payee, use hermes command to do it: hermes --config network/hermes/config.toml fee register-counterparty-payee --chain test-2 --channel channel-0 --port transfer --counterparty-payee $RLY_1
  5. For all transfers, instead of using simd tx ibc-transfer transfer transfer, use hermes transfer command, e.g. hermes --config network/hermes/config.toml fee transfer --dst-chain test-2 --src-chain test-1 --src-port transfer --src-channel channel-0 --amount 1000 --denom stake --receive-fee 50 --ack-fee 25 --timeout-fee 10 --key-name wallet1 --recipient $WALLET_3 --timeout-seconds 120

If we compare the balance of $RLY_1, $RLY_2, $WALLET_1 and $WALLET_2 before and after the transfer, the following should be true:

  • For the forward relayer $RLY_1, the final balance is: initial_balance - gas_fee + recv_fee (50) + ack_fee (25)
  • For the reverse relayer $RLY_2, the final balance is: initial_balance - gas_fee
  • For $WALLET_1 the balance is: initial_balance - transfer_amount (1000) - recv_fee (50) - ack_fee(25) - gas_fee
  • For $WALLET_2 the balance is the same for stake but has an additional 1000 ibc/<hash>

The gas fees can be retrieved by the logs in the terminal which called hermes start

Copy link
Contributor

@seanchen1991 seanchen1991 left a comment

Choose a reason for hiding this comment

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

Have a couple of questions from looking over the integration tests. I also attempted to add some cursory documentation for them.

@romac romac merged commit 2122813 into master Oct 24, 2022
@romac romac deleted the soares/ics29-fee-middleware branch October 24, 2022 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: ics29-fee Objective: Fee middleware support
Projects
None yet
5 participants