feat: entrypoint upgrade #9

New Issue

AtHeartEngineer · 2025-07-08T08:38:38-04:00

AtHeartEngineer commented

2025-07-08 08:38:38 -04:00

Originally created by @0xmoebius on 5/15/2025

This pull request introduces changes to the to-be-upgraded Entrypoint contract.
With this upgrade, a usedPrecommitments mapping is added to the contract, which tracks all used precommitments and prevents users from depositing using an already used precommitment, thus preventing stuck funds.

Introduced changes:

state variable and checks added to Entrypoint
added unit tests for the new revert case
added an upgrade test using a forked environment from Ethereum Mainnet

An issue of this kind could theoretically happen to the secrets used on withdrawals, though it’s way less probable because of the nature of the commitment system on withdrawals:

When withdrawing, partially or completely, a previous existing commitment is spent and a new one is created for the remaining value (even if it’s zero). Because of this, when generating the withdrawal proof, the user must provide the nullifier of the spending commitment and a new nullifier for the new commitment. The withdraw.circom circuit does take this in count and will never produce a withdrawal proof with a new commitment with the same nullifier as the just spent one.
The secrets for deposits and withdrawals are generated differently. Both are created deterministically based on a master_nullifier and master_secret, but the data used as pre-image of the ultimate secret values is inherently different. For deposits, the secrets are image of poseidon(master_key, pool_scope, deposit_index), while the ones of withdrawals are the image of poseidon(master_key, deposit_label, withdrawal_index). This makes the chances of collision of deposit and withdrawal secrets almost non-existent.
When withdrawing, since a commitment is spent, its nullifier is marked as spent and that is checked for all further commitments to avoid double-spending. If a user were to submit two partial withdrawals of the same origin deposit quickly with an outdated state, one withdrawal transaction would be successful and the second one would revert, as both withdrawals would be trying to spend the same commitment, which can only be spent once.

This issue can not be used by a third actor in a malicious way whatsoever. Another account can see the chain and use your same pre-commitment for a deposit, but only the user who owns the master keys generated by the seed-phrase is the one that will be able to later spend the commitment.

*Originally created by @0xmoebius on 5/15/2025* This pull request introduces changes to the to-be-upgraded `Entrypoint` contract. With this upgrade, a `usedPrecommitments` mapping is added to the contract, which tracks all used precommitments and prevents users from depositing using an already used precommitment, thus preventing stuck funds. Introduced changes: - state variable and checks added to Entrypoint - added unit tests for the new revert case - added an upgrade test using a forked environment from Ethereum Mainnet An issue of this kind could theoretically happen to the secrets used on withdrawals, though it’s way less probable because of the nature of the commitment system on withdrawals: 1. When withdrawing, partially or completely, a previous existing commitment is spent and a new one is created for the remaining value (even if it’s zero). Because of this, when generating the withdrawal proof, the user must provide the nullifier of the spending commitment and a new nullifier for the new commitment. The `withdraw.circom` [circuit does take this in count](https://github.com/0xbow-io/privacy-pools-core/blob/6b38184fec2ad8d85a5adf0796d30d06426db124/packages/circuits/circuits/withdraw.circom#L94) and will never produce a withdrawal proof with a new commitment with the same nullifier as the just spent one. 2. The secrets for deposits and withdrawals are generated differently. Both are created deterministically based on a `master_nullifier` and `master_secret`, but the data used as pre-image of the ultimate secret values is inherently different. For [deposits](https://github.com/0xbow-io/privacy-pools-core/blob/6b38184fec2ad8d85a5adf0796d30d06426db124/packages/sdk/src/core/account.service.ts#L241), the secrets are image of `poseidon(master_key, pool_scope, deposit_index)`, while the ones of [withdrawals](https://github.com/0xbow-io/privacy-pools-core/blob/6b38184fec2ad8d85a5adf0796d30d06426db124/packages/sdk/src/core/account.service.ts#L276) are the image of `poseidon(master_key, deposit_label, withdrawal_index)`. This makes the chances of collision of deposit and withdrawal secrets almost non-existent. 3. When withdrawing, since a commitment is spent, its nullifier is marked as spent and that is checked for all further commitments to avoid double-spending. If a user were to submit two partial withdrawals of the same origin deposit quickly with an outdated state, one withdrawal transaction would be successful and the second one would revert, as both withdrawals would be trying to spend the same commitment, which can only be spent once. This issue can not be used by a third actor in a malicious way whatsoever. Another account can see the chain and use your same pre-commitment for a deposit, but only the user who owns the master keys generated by the seed-phrase is the one that will be able to later spend the commitment.

AtHeartEngineer closed this issue

2025-07-08 08:38:39 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/privacy-pools-core#9