-
Notifications
You must be signed in to change notification settings - Fork 955
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
Store IBC trace when minting #3319
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3319 +/- ##
==========================================
- Coverage 53.89% 53.71% -0.18%
==========================================
Files 314 314
Lines 105704 105561 -143
==========================================
- Hits 56968 56702 -266
- Misses 48736 48859 +123 ☔ 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.
lgtm
impl TryFrom<Event> for IbcEvent { | ||
type Error = EventError; | ||
|
||
fn try_from(namada_event: Event) -> std::result::Result<Self, Self::Error> { | ||
Ok(Self { | ||
event_type: validate_ibc_event_type(&namada_event)?, | ||
attributes: { | ||
let mut attrs: HashMap<_, _> = | ||
namada_event.into_attributes().into_iter().collect(); | ||
attrs.with_attribute(event_domain_of::<Self>()); | ||
attrs | ||
}, | ||
}) | ||
} | ||
} | ||
|
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.
I think this could have still been useful, but not to the ledger codebase directly. it could be useful as sdk library code for users to consume. maybe this can be restored in the future, with proper event type validation
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.
ACK modulo nit
owner: &Address, | ||
coin: &PrefixedCoin, | ||
) -> Result<(), TokenTransferError> { | ||
if coin.denom.trace_path.is_empty() { |
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.
Why would we call the function in this case, though? If we must do it like this, we should call the function maybe_store_ibc_denom
or something, since it sometimes doesn't store anything at all.
* yuji/store-ibc-trace: fix func name add changelog new is_receiving_success store ibc traces when minting
* origin/yuji/store-ibc-trace: fix func name add changelog new is_receiving_success store ibc traces when minting
Describe your changes
closes #3317, closes #3229
Also,
get_ibc_events
and IBC event conversion functionsis_receiving_success
without checking eventsIndicate on which release or other PRs this topic is based on
v0.37.0
Checklist before merging to
draft