axelarnetwork / axelar-gmp-sdk-solidity

Solidity libraries and utilities provided by Axelar.
27 stars 30 forks source link

feat: create gmp tracker event interface #186

Open SGiaccobasso opened 2 weeks ago

codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.86%. Comparing base (febcd9f) to head (1cd9af6). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #186 +/- ## ========================================== + Coverage 98.54% 99.86% +1.31% ========================================== Files 48 48 Lines 758 756 -2 Branches 157 157 ========================================== + Hits 747 755 +8 Misses 1 1 + Partials 10 0 -10 ``` | [Flag](https://app.codecov.io/gh/axelarnetwork/axelar-gmp-sdk-solidity/pull/186/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=axelarnetwork) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/axelarnetwork/axelar-gmp-sdk-solidity/pull/186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=axelarnetwork) | `99.86% <ø> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=axelarnetwork#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

canhtrinh commented 1 week ago
  1. For our RFQ, sender won't be on the chain that this event is being emitted, the sender of the transaction being emitted will just be a relayer for our settlement process, so this should be a string. Could be a sender from Sui, Sol, or Cosmos. => Thanks, yeah we'll change to a string

  2. We don't have decimals available, so wouldn't be able to emit that. => How do you represent amounts then?

  3. Swaps have a fromToken and toToken, so a bridge amount and single tokenAddress is less relevant here. Better to have fromToken, fromAmount, toToken and toAmount for us. We could arbitrarily pick the destination token if you want just one. => Hmm, yeah we just want one. I'll want some team buy-in, but perhaps we can accept both from and to so you don't have to pick

ecdsafu commented 1 week ago
  1. For our RFQ, sender won't be on the chain that this event is being emitted, the sender of the transaction being emitted will just be a relayer for our settlement process, so this should be a string. Could be a sender from Sui, Sol, or Cosmos.

=> Thanks, yeah we'll change to a string

  1. We don't have decimals available, so wouldn't be able to emit that.

=> How do you represent amounts then?

  1. Swaps have a fromToken and toToken, so a bridge amount and single tokenAddress is less relevant here. Better to have fromToken, fromAmount, toToken and toAmount for us. We could arbitrarily pick the destination token if you want just one.

=> Hmm, yeah we just want one. I'll want some team buy-in, but perhaps we can accept both from and to so you don't have to pick

  1. Sounds good, thanks

  2. We have off chain mappings for the token/chain/decimals and amount is always represented in the decimals of that token/chain

  3. Ok sounds good, if you just want one we can choose only one, will leave it with you if you want to add the second token or not

ecdsafu commented 5 days ago

Looking good, for our RFQ we will always emit from source on the hub chain. If you add decimals back in, we won't be able to support anyway and would leave at 0