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

Commit pending state changes before updating client in testing pkg #3900

Open
3 tasks
colin-axner opened this issue Jun 19, 2023 · 3 comments
Open
3 tasks
Labels
needs discussion Issues that need discussion before they can be worked on testing Testing package and unit/integration tests

Comments

@colin-axner
Copy link
Contributor

Summary

The testing package has a NextBlock function which commits the current block and begins the next block. It should commit to the current working app hash. Currently it commits to the app hash which was set when the block had begun, causing any writes to the block to require 2 commits to be proved (one commit to clear the current block and a second to make the state changes provable)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added the testing Testing package and unit/integration tests label Jun 19, 2023
@colin-axner
Copy link
Contributor Author

see #3846 (comment)

@colin-axner
Copy link
Contributor Author

We can likely take the app hash from FinalizeBlock in the eden upgrade, see here

I tried using the chain.App.LastCommitID().Hash after calling chain.App.Commit() in the existing code, but this breaks proof verification for some reason? Need to investigate some more, but I believe this might actually be expected behaviour. Since I believe cometbft commits to a header before executing state changes, so state changes are actually reflected in the next signed block header

@colin-axner colin-axner changed the title Within ibc testing NextBlock, commit the updated AppHash Commit pending state changes before updating client in testing pkg Jul 4, 2023
@colin-axner
Copy link
Contributor Author

This issue should produce two blocks before an update client (one to commit the currently proposed header and one to make available the pending state changes produced by tests), it might be possible to remove one commit if you can see if the working hash == latest commit hash (but this would be an optimization, maybe we could leave a descriptive comment to explain the behaviour)

@crodriguezvega crodriguezvega moved this to Backlog in ibc-go Nov 6, 2023
@colin-axner colin-axner added the needs discussion Issues that need discussion before they can be worked on label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issues that need discussion before they can be worked on testing Testing package and unit/integration tests
Projects
Status: Backlog 🕐
Development

No branches or pull requests

1 participant