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

Miscellaneous spec updates from review (mostly ICS 20) #375

Merged
merged 17 commits into from
Feb 27, 2020

Conversation

cwgoes
Copy link
Contributor

@cwgoes cwgoes commented Feb 3, 2020

@cwgoes cwgoes added tao Transport, authentication, & ordering layer. from-review Feedback / alterations from specification review. labels Feb 3, 2020
@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 6, 2020

Refactoring ICS 20 (spec & impl) to correctly handle acknowledgements, my current thought is that application-level specs should always use acknowledgement data to indicate failure instead of aborting the transaction (which is also possible). The reason I think this is preferable is that the acknowledgement data can contain detailed information about the failure, so that the original sender of the packet can handle it appropriately (this may mean different things based on the kind of packet & kind of failure - e.g. if the destination address was invalid, refund the tokens, but if the denomination was invalid, something went wrong with the version negotiation process, if there aren't enough tokens, there has been some sort of Byzantine behaviour somewhere in the system that needs to be handled, etc.)

Just aborting the transaction can't provide this because it's only 1 bit of data (success/failure) and can't communicate the difference between different kinds of failures which might need to be handled differently. Writing acknowledgements will require a little bit more state usage, but not much, and it seems worthwhile. Relayers can still elect not to relay acknowledgements of success (which are no-ops on the sending chain) in the optimistic all-ok path.

Does this make sense as a design pattern? Anything I missed?

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 19, 2020

  • Also deal with source/dest channel/port arguments required in createOutgoingPacket here

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 20, 2020

Per discussion with @fedekunze:

  • Alter spec so that whether or not we are the source chain is deduced from the denom
  • Note a bit about future registries of "root chain" for each denom & supply accounting

@cwgoes cwgoes changed the title Start updating ICS 20 Miscellaneous spec updates from review (mostly ICS 20) Feb 24, 2020
@cwgoes cwgoes marked this pull request as ready for review February 24, 2020 11:28
spec/ics-020-fungible-token-transfer/README.md Outdated Show resolved Hide resolved
spec/ics-020-fungible-token-transfer/README.md Outdated Show resolved Hide resolved
spec/ics-020-fungible-token-transfer/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from-review Feedback / alterations from specification review. tao Transport, authentication, & ordering layer.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants