feat: added a cosmos-to-cosmos relayer module with basic tests
#170
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #170 +/- ##
=======================================
Coverage 96.83% 96.83%
=======================================
Files 11 11
Lines 537 537
=======================================
Hits 520 520
Misses 17 17 ☔ View full report in Codecov by Sentry. |
srdtrk
changed the title
cosmos-to-cosmos relayer module (only chain info)cosmos-to-cosmos relayer module with basic tests
| }) | ||
| .map(|e| async { | ||
| match e { | ||
| EurekaEvent::SendPacket(se) => { |
There was a problem hiding this comment.
To make this a bit more readable and less indented, I think it might make sense to pull out the event handling into a separate function like handle_send_packet_timeout or something like more precise if you prefer.
There was a problem hiding this comment.
Yes, this whole function is too long. I will refactor this once it is actually working. No need to create an issue I think since I wouldn't forget this as it is very annoying.
| let _recv_msgs = future::try_join_all(src_send_events.into_iter().map(|e| async { | ||
| match e { | ||
| EurekaEvent::SendPacket(se) => { | ||
| let ibc_path = se.packet.commitment_path(); |
There was a problem hiding this comment.
Same here, I think it would be nice to pull this into a separate function
There was a problem hiding this comment.
Yes, this whole function is too long. I will refactor this once it is actually working. No need to create an issue I think since I wouldn't forget this as it is very annoying.
| let _ack_msgs = future::try_join_all(src_ack_events.into_iter().map(|e| async { | ||
| match e { | ||
| EurekaEvent::WriteAcknowledgement(we) => { | ||
| let ibc_path = we.packet.ack_commitment_path(); |
There was a problem hiding this comment.
Yes, this whole function is too long. I will refactor this once it is actually working. No need to create an issue I think since I wouldn't forget this as it is very annoying.
| #[async_trait::async_trait] | ||
| impl TxBuilderService<CosmosSdk, CosmosSdk> for TxBuilder { | ||
| #[allow(clippy::too_many_lines)] | ||
| async fn relay_events( |
There was a problem hiding this comment.
In general, I think this function should be split up more. I would say separate functions for each type of tx, or potentially target vs src events you are looking for + each event handling.
There was a problem hiding this comment.
I think this should also be done for eth_eureka, probably, but you could do that in a separate PR if you prefer.
There was a problem hiding this comment.
Yes, this whole function is too long. I will refactor this once it is actually working. I can create an issue for this
| .chain_id() | ||
| .await | ||
| .map_err(|e| tonic::Status::from_error(e.to_string().into()))?, | ||
| ibc_version: "2".to_string(), |
There was a problem hiding this comment.
I guess we only support version 2, do we have any checks anywhere to fail during setup if version 1 comes in?
There was a problem hiding this comment.
Not sure what I wanna do with this field. Can remove if you'd like
| @@ -100,7 +99,7 @@ impl RelayerService for CosmosToEthRelayerModuleServer { | |||
| .await | |||
| .map_err(|e| tonic::Status::from_error(e.to_string().into()))?, | |||
| ibc_version: "2".to_string(), | |||
There was a problem hiding this comment.
Should this also be like an enum or number or something? Feels a bit strange to use a string for this
There was a problem hiding this comment.
I think this interface isn't finalized enough yet to warrant an enum. Might even fully remove it.
There was a problem hiding this comment.
Ok, makes sense. When things are a bit more finalized, we can do a review again of the whole thing to make sure we don't miss stuff like this 👍
|
LGTM! 🚀 |
Description
closes: #153
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.