Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

Withdraw roots colliding #664

Open
dmaretskyi opened this issue Sep 14, 2021 · 1 comment
Open

Withdraw roots colliding #664

dmaretskyi opened this issue Sep 14, 2021 · 1 comment
Labels
bug Something isn't working contracts This PR changes some contracts p2

Comments

@dmaretskyi
Copy link

It look like WithdrawManager stores pending withdrawals using withdrawRoot which is the root of the merkle tree formed by new user states generated after mass-migration batch.

The states are formed without a nonce:

    Types.UserState memory state =
            Types.UserState({
                pubkeyID: pubkeyID,
                tokenID: tokenID,
                balance: amount,
                nonce: 0
            });

In that case, does this allow for a batch to be forged in a way that will cause a collision in withdrawRoot?

https://github.dev/thehubbleproject/hubble-contracts/blob/7058cba1a4e5a6251571f74a6a7f16de5b77e2a7/contracts/libs/Transition.sol#L217-L217

https://github.dev/thehubbleproject/hubble-contracts/blob/7058cba1a4e5a6251571f74a6a7f16de5b77e2a7/contracts/WithdrawManager.sol#L47-L47

@jacque006 jacque006 added bug Something isn't working contracts This PR changes some contracts question Further information is requested labels Sep 14, 2021
@dmaretskyi
Copy link
Author

The simplest case where such a collision might happen is two MM commitments, each with a single transaction. In that case withdraw root for each of them would be calculated with:

withdrawRoot = keccak256(Types.UserState({
    pubkeyID: pubkeyID,
    tokenID: tokenID,
    balance: amount,
    nonce: 0
}).encode());

Now, as long as pubkey id, token id, and amount are the same in both transactions, which is a valid case, then the withdraw roots would be the same for both commitments.

@jacque006 jacque006 added p2 and removed question Further information is requested labels Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working contracts This PR changes some contracts p2
Projects
None yet
Development

No branches or pull requests

2 participants