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

IBC Spec conformity analysis #97

Open
gjermundgaraba opened this issue Nov 13, 2024 · 3 comments
Open

IBC Spec conformity analysis #97

gjermundgaraba opened this issue Nov 13, 2024 · 3 comments

Comments

@gjermundgaraba
Copy link
Contributor

To make sure the Solidity implementation follows the IBC Eureka spec, we need to do an analysis of the current implementation and see where we might be diverging from the spec. Once we have done that, we can go through those items and make plans to change them, or, if there are good reasons not to, document them.

@sangier
Copy link
Contributor

sangier commented Nov 15, 2024

E.g. in the solidity onRecvPacket the error handling is not aligned with the spec as it use errorAcks differently.

@srdtrk
Copy link
Member

srdtrk commented Nov 18, 2024

Can you elaborate about error acks @sangier ?

@sangier
Copy link
Contributor

sangier commented Nov 18, 2024

Sure! E.g. In ICS20v1 in the onRecv :

  • we revert if the inputs data are not valid
  • use error ack in case the bank module errors out.

In the solidity implementation:

  • we error ack if some of the inputs are invalid or not resolved properly
  • we don't handle gracefully the potential failure of escrow send and escrow mint (equivalent of bank), so in case of failure we would automatically revert.

There may be other cases like that, I haven't fully checked the code yet. Anyway that's an opportunity to clarify explicitly how we wanna implement the error handling behaviour: when to revert, when to error ack? why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

3 participants