Skip to content
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

Determine changes to the Component trait needed for Astria to use it directly #3126

Closed
Tracked by #3125
hdevalence opened this issue Sep 30, 2023 · 5 comments
Closed
Tracked by #3125
Assignees

Comments

@hdevalence
Copy link
Member

As part of #3125, we want Astria to be able to use the penumbra-ibc crate as their IBC implementation.

For Astria to use the penumbra-ibc Component implementation, it should be using the exact same Component trait as the other Astria components. In the design meeting last Thursday, it sounded like the Astria team had forked the Component trait to make some tweaks to it (largely about error modeling?).

As a first step, we should collect feedback from the Astria team about changes to the Component trait and then align both projects on a common interface.

cc @noot

@hdevalence
Copy link
Member Author

@noot I would have assigned this issue to you but I think I can't do that since you're out of the penumbra-zone org; can you self-assign?

We can pick up making the required changes on our side once we align on what they are.

@noot
Copy link
Collaborator

noot commented Oct 6, 2023

@hdevalence unfortunately I can't self assign either :(

here is the astria Component trait: https://github.com/astriaorg/astria/blob/main/crates/astria-sequencer/src/component.rs

I think the main difference is we removed end_epoch since we aren't using epochs at the moment. we also added Result returns to all the methods, although I don't think that's strictly necessary (as we can just panic inside the function, because either way the app is going to go down). the main motivation for adding the Results is just so the error can be bubbled up before the app goes down, but I don't feel strongly about that.

@aubrika aubrika self-assigned this Oct 18, 2023
@aubrika aubrika removed their assignment Oct 25, 2023
@hdevalence
Copy link
Member Author

@noot Can you clarify if we actually need to do this in order for y'all to use the penumbra-ibc component? If (like us) you're not actually writing code that's generic over components, and just using it as a way to have common behavior, this might not be on the critical path, correct?

@noot
Copy link
Collaborator

noot commented Nov 28, 2023

@hdevalence i don't think we need this - i've added the IBC code needed for ICS20 transfers and this hasn't come up, so should be fine as-is!

@avahowell
Copy link
Contributor

Closing this then, since tentatively the current component trait design should work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Status: Testnet 63: Rhea (Web Wallet)
Development

No branches or pull requests

4 participants