feat: use abi encoding of ICS20Transfer payload #117
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #117 +/- ##
==========================================
- Coverage 94.10% 93.85% -0.25%
==========================================
Files 9 9
Lines 339 309 -30
==========================================
- Hits 319 290 -29
+ Misses 20 19 -1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| @@ -10,6 +10,9 @@ on: | |||
| push: | |||
| branches: | |||
| - "main" | |||
| paths: | |||
There was a problem hiding this comment.
not really related to this, so can take it out. I just figure it would be good limit runs of this when it's not necessary.
| @@ -0,0 +1,42 @@ | |||
| module github.com/cosmos/solidity-ibc-eureka/abigen | |||
|
|
|||
| go 1.22.7 | |||
There was a problem hiding this comment.
Using the setup as the e2e tests. But maybe we should just fully update it to go 1.23 soon...
| Repository: "ghcr.io/cosmos/ibc-go-wasm-simd", // FOR LOCAL IMAGE USE: Docker Image Name | ||
| Version: "feat-ibc-eureka", // FOR LOCAL IMAGE USE: Docker Image Tag | ||
| Repository: "ghcr.io/cosmos/ibc-go-wasm-simd", // FOR LOCAL IMAGE USE: Docker Image Name | ||
| Version: "gjermund-7591-ics20-abi-encoding", // FOR LOCAL IMAGE USE: Docker Image Tag |
There was a problem hiding this comment.
This one can only be updated once cosmos/ibc-go#7592 is merged. We could wait, but I'd rather just update this in a PR so we're not blocked on ibc-go reviewing which might take time.
| @@ -39,12 +44,14 @@ generate-abi: | |||
| jq '.abi' out/ERC20.sol/ERC20.json > abi/ERC20.json | |||
| jq '.abi' out/IBCERC20.sol/IBCERC20.json > abi/IBCERC20.json | |||
| jq '.abi' out/IBCStore.sol/IBCStore.json > abi/IBCStore.json | |||
| abigen --abi abi/ICS02Client.json --pkg ics02client --type Contract --out e2e/interchaintestv8/types/ics02client/contract.go | |||
There was a problem hiding this comment.
Mostly just different ordering to have some level of separation between types generated for e2e tests only and the ones that should be public in abigen/.
That, and adding ICS20Lib.
|
A quick comment on the test coverage. I believe it went down a little bit because some of the old parsing logic was well tested... I looked over the main parts of the ICS20Lib that has some branches that are not unit tested right now, and it does not seem worth spending time on them, as they will all be replaced by new functions from OpenZeppelin in their next release (which should be early next year it looks like). |
Description
closes: #116
Based off of the work done in: #102
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.