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

feat(ITS): add ITS message ABI encoding/decoding support #65

Merged
merged 18 commits into from
Nov 21, 2024

Conversation

AttissNgo
Copy link
Contributor

@AttissNgo AttissNgo commented Nov 13, 2024

@AttissNgo AttissNgo requested a review from a team as a code owner November 13, 2024 01:30
@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 90.76577% with 41 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@527c2fb). Learn more about missing BASE report.

Files with missing lines Patch % Lines
contracts/interchain-token-service/src/abi.rs 90.76% 41 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #65   +/-   ##
=======================================
  Coverage        ?   93.60%           
=======================================
  Files           ?       33           
  Lines           ?     2018           
  Branches        ?        0           
=======================================
  Hits            ?     1889           
  Misses          ?      129           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Contributor

@ahramy ahramy left a comment

Choose a reason for hiding this comment

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

ABI encoding/decoding could be quite challenging, but you managed it very well. Well done!

contracts/interchain-token-service/Cargo.toml Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/lib.rs Show resolved Hide resolved
contracts/interchain-token-service/src/types.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
- add ABI encoding/decoding for ITS messages
- add ITS Hub message types
- replace match statements with one-liners where appropriate
- import std in root lib.rs #[cfg(test)] in order to use goldie assertions
- add error handling to `try_into()`
- add `into_vec` and `from_vec` helpers
- rename soroban string and i128 conversion functions
- remove redundant unit test
- change HubMessage to standard enum (not #[contracttype]) and refactor
- simplify `to_i128` conversion function and use `ensure!` macro
@AttissNgo AttissNgo force-pushed the feat/its-message-abi-encoding branch from 5ff1615 to f9e5ac8 Compare November 18, 2024 20:39
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Show resolved Hide resolved
- add checks for negative values for Uint<256,4> to i128 conversion
- add test for invalid UTF-8 sequence when converting to Std String
- add test for i128 overflow
- remove #[contracttype] for all ITS message types
- change decimals to u8
Cargo.toml Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
- change abi_decode method to have single point of exit (no early return for error)
- add u128 overflow test for `to_i128()`
- add improved error handling for string conversion (return Result instead of panic)
- improved tests for string conversion
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Outdated Show resolved Hide resolved
contracts/interchain-token-service/src/abi.rs Show resolved Hide resolved
- remove unnecessary test
- use assert_ok! in tests rather than unwrap
- add test case for invalid hub message type
@AttissNgo AttissNgo merged commit 9c49a73 into main Nov 21, 2024
5 checks passed
@AttissNgo AttissNgo deleted the feat/its-message-abi-encoding branch November 21, 2024 21:04
@ahramy ahramy mentioned this pull request Jan 21, 2025
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

Successfully merging this pull request may close these issues.

5 participants