-
Notifications
You must be signed in to change notification settings - Fork 94
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
Signer parsing from context #1393
Signer parsing from context #1393
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1393 +/- ##
==========================================
+ Coverage 67.07% 67.13% +0.05%
==========================================
Files 226 226
Lines 22485 22455 -30
==========================================
- Hits 15082 15075 -7
+ Misses 7403 7380 -23 ☔ View full report in Codecov by Sentry. |
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.
Thanks for splitting out this PR! Please take a look at the comments. Additionally:
- Update the PR with the main branch.
- Apply the same changes to the https://github.com/cosmos/ibc-rs/blob/ede380fde4cd02fb8d66ebf7a9c746af50ddfc97/ibc-apps/ics721-nft-transfer/src/context.rs#L38
12ffb4b
to
76b9943
Compare
76b9943
to
6f5becd
Compare
6f5becd
to
5115168
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.
🎉
a5f9fbf
Based on #1392
Diff: https://github.com/heliaxdev/cosmos-ibc-rs/compare/heliaxdev:cosmos-ibc-rs:tiago/optional-ack-rebased..heliaxdev:cosmos-ibc-rs:tiago/signer-parsing-from-ctx
Description
This PR replaces the
TryFrom<Signer>
bound onAccountId
with new context methods, with the aim of contextually parsingSigner
instances.(Some context: #1392 (comment))
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.