-
Notifications
You must be signed in to change notification settings - Fork 329
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
Remove relayer-specific code from modules #2447
Conversation
e14c382
to
ec7256c
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.
LGTM
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 Shoaib!
This changes no logic from what I can tell. Looks good!
const HEIGHT_ATTRIBUTE_KEY: &str = "height"; | ||
const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; | ||
const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; | ||
const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; | ||
const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; | ||
const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; | ||
pub const HEIGHT_ATTRIBUTE_KEY: &str = "height"; | ||
pub const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; | ||
pub const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; | ||
pub const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; | ||
pub const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; | ||
pub const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; |
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.
These are Tendermint-specific, so we might move them out of ibc
crate later on.
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.
Right, I was also thinking about these and I didn't remove them for now because basecoin needs (and arguably other chains using the modules would need) conversions from IbcEvent
to tendermint_abci::Event
(which use these const
s). We need to figure out a good place for these conversions.
I wonder if it makes sense to merge this directly into master? The changes are not specific to light client extraction and the diff wouldn't change as hu55a1n1/light-client-extraction is in sync with |
Yes, good point! Let's merge directly to |
Before that, can we drop the |
* Move relayer specific module events code into relayer crate * Fix relayer imports * Remove `_events` suffix from `events::` submodules * Fix changelog entry for informalsystems#2431 * Add changelog entry
Partially addresses: #2335
Description
This PR moves relayer-specific code from the modules crate into the relayer crate
Note: The target branch is a long-lived branch hu55a1n1/light-client-extraction. The plan is to merge PRs (addressing #2335) such as this into hu55a1n1/light-client-extraction and ultimately merge that into master.
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.