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

Simplify reverts of forwarding state on acknowledgement #6558

Closed
3 tasks
DimitrisJim opened this issue Jun 10, 2024 · 6 comments · Fixed by #6574
Closed
3 tasks

Simplify reverts of forwarding state on acknowledgement #6558

DimitrisJim opened this issue Jun 10, 2024 · 6 comments · Fixed by #6574
Assignees
Labels
20-transfer type: feature New features, sub-features or integrations

Comments

@DimitrisJim
Copy link
Contributor

Investigate if we can simplify revert logic by doing regular refund packet tokens first and then the logic for revert forwarding changes.

Surfaced during internal walkthrough of ics20 v2 forwarding (commit efcfa5d)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

Looks like it should be possible. Opened cosmos/ibc#1110 to update the spec.

@crodriguezvega crodriguezvega added the type: feature New features, sub-features or integrations label Jun 10, 2024
@crodriguezvega crodriguezvega moved this to Todo 🏃 in ibc-go Jun 10, 2024
@srdtrk
Copy link
Member

srdtrk commented Jun 11, 2024

I can take this if others agree

@DimitrisJim
Copy link
Contributor Author

absolutely, go for it

@srdtrk
Copy link
Member

srdtrk commented Jun 12, 2024

There was a suggestion to remove the state stored in GetForwardedPacket after revert logic is done. Should I create a new issue for this or address it in #6474?

I think we could also switch to using a collections item. Could also create an issue for that

@DimitrisJim
Copy link
Contributor Author

DimitrisJim commented Jun 12, 2024

I think follow up would be better? No need to add to diff.

Also, re collection item, should add it as an additional thing to migrate in #6404. Think it makes sense to just reword that issue to mention migrating all transfer state to collections.

@srdtrk
Copy link
Member

srdtrk commented Jun 13, 2024

closed by #6574

@srdtrk srdtrk closed this as completed Jun 13, 2024
@github-project-automation github-project-automation bot moved this from Todo 🏃 to Done 🥳 in ibc-go Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20-transfer type: feature New features, sub-features or integrations
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants