-
Notifications
You must be signed in to change notification settings - Fork 28
Disallow calling processWithdrawCommitment twice #679
base: master
Are you sure you want to change the base?
Disallow calling processWithdrawCommitment twice #679
Conversation
I feel that this issue should be resolved together with #664 and we would come up with a better solution then. |
I agree with @msieczko , resolving #664 would be a better path forward. @duckception Appreciate you taking a stab at trying to resolve this. |
Issue described in #664 should be now resolved 🚀 Additionally, I have a question @jacque006. What is the purpose of |
254757d
to
6e524a2
Compare
We've discovered that changing just the nonce is not enough to prevent withdraw roots collision. Hubble allows registration of multiple user states for the same pubkey ID, therefore there still can be a case where withdraw root collision could happen (see modified test in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good now! @jacque006, could you take a look? I think this PR is ready to be merged. We should also merge #684 right after this one.
Currently, anyone can call
processWithdrawCommitment
multiple times which may lead to permanent lock up of tokens in theWithdrawManager
. This PR fixes it and removes redundant code from theVault
smart contract.