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

remove ERC addresses from transfer #509

Merged
merged 1 commit into from
Feb 23, 2022
Merged

Conversation

Westlad
Copy link
Contributor

@Westlad Westlad commented Feb 17, 2022

fixes #497

This fixes a bug whereby transfer transactions advertise the ERCAddress of the token being transferred. Allowing this would greatly reduce the size of the annonymity set, and it's unnecessary because the Shield contract doesn't need to know it. Hence we make ERCAddress a private variable in the ZKP circuit.

To test: testing is the conventional ./start-nightfall -g [-s], npm t approach BUT because the circuits have changed, you must delete your proving_files volume to force a new trusted setup and to start using the new circuits.

When the test has run, and before you stop nightfall, you can do docker-compose logs | grep -A 4 'transactionType": "1"' to see the single transfer transactions and the ERCAddress contained therein (should be zeroed out). Replace "1" with "2" to see the double transfers.

@ChaitanyaKonda ChaitanyaKonda added the One more approval needed One reviewer has approved this PR but another is needed label Feb 18, 2022
@IlyasRidhuan IlyasRidhuan merged commit 2ce68f1 into master Feb 23, 2022
@IlyasRidhuan IlyasRidhuan deleted the westlad/ercaddress-removal branch February 23, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
One more approval needed One reviewer has approved this PR but another is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ercAddress from transaction calldata for single and double transfers
3 participants