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

Debug why WirePayForMessages are not removed from the mempool when rechecking transactions #584

Closed
evan-forbes opened this issue Nov 21, 2021 · 4 comments

Comments

@evan-forbes
Copy link
Member

Summary

In #581, we introduced a temporary fix to remove WirePayForMessages from the mempool. For a long term solution, we need to debug why the transactions are not removed when rechecking the transactions in the mempool after they are committed.

Details

The temporary fix introduced in #582 relies on a wrapped transaction type that attaches the hash of the parent WirePayForMessage transaction to the child MsgPayForMessage. This hash is then used to identify and remove the old WirePayForMessage from the mempool after the MsgPayForMessage is committed to.

This solution has a lot of downsides, and is only meant to patch the bug until we can debug why the existing solution isn't working as expected.

References

Hacky solution issue #581, core PRs #582 and #583, along with the app PR #149

@adlerjohn
Copy link
Member

Can you clarify what a "parent" tx and the parent tx hash vs the current (?) tx hash is?

@evan-forbes
Copy link
Member Author

Whoops, missed this. We might have covered it synchronously, but for posterity

Can you clarify what a "parent" tx and the parent tx hash vs the current (?) tx hash is?

parent tx is the original WirePayForMessage submitted by the user, the child tx is the PayForMessage tx that is derived from the parent WirePayForMessage tx.

The hashes are obvi different, but tendermint relies on them being the same, as the mempool uses the hash as a key. After a child tx is confirmed to be included in a block, the parent was not being removed from the mempool, and the user is not updated to let them know that their transaction has been included in a block

@evan-forbes
Copy link
Member Author

evan-forbes commented Jun 14, 2022

Heyo, this was likely the reason why this was happening even after the hacky fix cosmos/cosmos-sdk#12060

@evan-forbes
Copy link
Member Author

I think we can close this since our release of v0.46.0 of the sdk has the above fix, and will be included after we hardfork mamaki

Repository owner moved this from TODO to Done in Celestia Node Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

No branches or pull requests

3 participants