-
Notifications
You must be signed in to change notification settings - Fork 960
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
IBC Packet Forward Middleware #4082
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4082 +/- ##
==========================================
- Coverage 74.58% 74.51% -0.08%
==========================================
Files 341 342 +1
Lines 107484 107894 +410
==========================================
+ Hits 80167 80396 +229
- Misses 27317 27498 +181 ☔ View full report in Codecov by Sentry. |
0cf7093
to
dc3950c
Compare
Co-authored-by: Jacob Turner <[email protected]>
Co-authored-by: Jacob Turner <[email protected]>
Co-authored-by: Jacob Turner <[email protected]>
(before rebase/history cleanup: dc3950c) |
dc3950c
to
c9c8d57
Compare
Co-authored-by: Jacob Turner <[email protected]>
Co-authored-by: Jacob Turner <[email protected]> Co-authored-by: Yuji Ito <[email protected]>
Co-authored-by: Jacob Turner <[email protected]>
c9c8d57
to
7b6135d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left trivial comments.
@@ -118,9 +119,10 @@ fs_extra = "1.2.0" | |||
futures = "0.3" | |||
git2 = { version = "0.18.1", default-features = false } | |||
# branch yuji/derive-arbitrary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating the branch name would be helpful
@@ -534,7 +529,7 @@ pub fn make_hermes_config(test_a: &Test, test_b: &Test) -> Result<()> { | |||
Ok(()) | |||
} | |||
|
|||
fn make_hermes_chain_config(test: &Test) -> Value { | |||
fn make_hermes_chain_config(_hermes_dir: &TestDir, test: &Test) -> Value { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove hermes_dir
?
&test_gaia_2, | ||
)?; | ||
|
||
// Check the samoleans have been sent from the first cosmos chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong comment here I think
self.transfer_module.ctx.inner.clone(), | ||
self.transfer_module.ctx.verifiers.clone(), | ||
); | ||
self.transfer_module.ctx.insert_verifier(&MULTITOKEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this also for send_transfer_execute
?
Describe your changes
Integrate with the Packet Forward Middleware code. Requires an
ibc-rs
fork, in order to support asynchronous IBC packet acknowledgements.NOTE: The code in the links above should also be reviewed.
Checklist before merging
breaking::
labelsnamada-docs
reponamada-indexer
ornamada-masp-indexer
, a corresponding PR is opened in that repo