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

Increase the size of the deposit output for better tracking-resistance #104

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

jimthematrix
Copy link
Contributor

@jimthematrix jimthematrix commented Nov 25, 2024

Previously, once deposited, the single output UTXO's value is known because it's equivalent to the amount of the deposit. This PR makes the output to be an array, currently set to size 2 in the circuit.

Copy link
Contributor

@Chengxuan Chengxuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a question about PTAU file changes

Copy link
Contributor

@Chengxuan Chengxuan Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimthematrix I don't feel the changes in the PR should have resulted in these PTAU changes. If you've upgraded Circom to v2.2.0 recently, that's likely to be the reason. adding --O2 will keep the same full form constraint simplification that we've been using and remove the need to update PTAU files as we default to using groth16. that's done as part of https://github.com/hyperledger-labs/zeto/pull/102/files#diff-9450c6f105ae987e3136ef62c143ea6d1baefd8a02e45bf3307fc0f781bdff01R154

I think for this PR, the PTAU file changes are not needed as the circom files are not on version v2.2.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for catching that. those are indeed resulted from the circom 2.2.1 update in my local environment, and not meant to be included. will revert that in a subsequent commit

@jimthematrix jimthematrix merged commit 6f49f1e into main Nov 27, 2024
6 checks passed
@jimthematrix jimthematrix deleted the deposit-outputs branch November 27, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants