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

refactor: final token cleanup #9864

Merged
merged 9 commits into from
Nov 12, 2024
Merged

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented Nov 9, 2024

In this PR I finally nuke the old shielding functionality of a token.

Copy link
Contributor Author

benesjan commented Nov 9, 2024

@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from c7e984b to dcbcd38 Compare November 11, 2024 14:54
@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch 3 times, most recently from 7267578 to 63bc42a Compare November 11, 2024 16:25
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from d1b498f to 0752c7a Compare November 11, 2024 17:43
@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch 3 times, most recently from b72f38b to 1680e4f Compare November 11, 2024 19:03
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from 3f31918 to 90ecd6f Compare November 11, 2024 19:48
@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch from b2e3360 to 1ba2129 Compare November 11, 2024 19:48
@@ -111,6 +103,7 @@ contract Token {
// docs:end:initialize_decimals
}
// docs:end:constructor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was bothering me that there were no spaces between functions so I sneaked it in.

@@ -472,8 +428,7 @@ contract Token {
// Transfers token `amount` from public balance of message sender to a private balance of `to`.
#[private]
fn transfer_to_private(to: AztecAddress, amount: Field) {
// We check the minter permissions in the enqueued call as that allows us to avoid the need for `SharedMutable`
// which is less efficient.
// `from` is the owner of the public balance from which we'll subtract the `amount`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment was incorrectly copied by me in a previous PR.

@@ -222,8 +222,7 @@ export class BlacklistTokenContractTest {
this.logger.verbose(`Public balance of wallet 0: ${publicBalance}`);
expect(publicBalance).toEqual(this.tokenSim.balanceOfPublic(address));

tokenSim.mintPrivate(amount);
tokenSim.redeemShield(address, amount);
tokenSim.mintPrivate(address, amount);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tokenSim.redeemShield was misused here and in other places as a way to increase a private balance of someone.

@@ -83,21 +83,18 @@ describe('e2e_blacklist_token_contract mint', () => {
});

describe('Mint flow', () => {
it('mint_private as minter', async () => {
it('mint_private as minter and redeem as recipient', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to merge these 2 tests as now it's not supported by the token simulator to have the shield flow in 2 steps.

@benesjan benesjan marked this pull request as ready for review November 11, 2024 20:15
Copy link
Contributor

github-actions bot commented Nov 11, 2024

Changes to public function bytecode sizes

Generated at commit: 34db5b218d4a81e061a66c5668b2493a0174ed64, compared to commit: e305f488b1502630f299bb03cf169770f2f6af09

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
AvmTest::public_dispatch -754 ✅ -1.22%
AvmTest::bulk_testing -1,724 ✅ -6.76%
Token::public_dispatch -2,716 ✅ -7.36%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
AvmTest::public_dispatch 61,247 (-754) -1.22%
AvmTest::bulk_testing 23,785 (-1,724) -6.76%
Token::public_dispatch 34,189 (-2,716) -7.36%

Base automatically changed from 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging to master November 12, 2024 13:40
@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch from 8b3278a to 0b19ee1 Compare November 12, 2024 14:23
@AztecBot
Copy link
Collaborator

AztecBot commented Nov 12, 2024

Docs Preview

Hey there! 👋 You can check your preview at https://6733c69cba91136a7aecf540--aztec-docs-dev.netlify.app

@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch from 0b19ee1 to d14b00e Compare November 12, 2024 15:55
@benesjan benesjan requested a review from sklppy88 November 12, 2024 15:56
@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch from d14b00e to 955f2e0 Compare November 12, 2024 18:06
Copy link
Contributor

@sklppy88 sklppy88 left a comment

Choose a reason for hiding this comment

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

looks great, love it! 🙌

@benesjan benesjan force-pushed the 11-09-refactor_final_token_cleanup branch from 74ddb10 to b8eb9ce Compare November 12, 2024 20:50
@benesjan benesjan enabled auto-merge (squash) November 12, 2024 20:50
@benesjan benesjan merged commit a6c1160 into master Nov 12, 2024
67 checks passed
@benesjan benesjan deleted the 11-09-refactor_final_token_cleanup branch November 12, 2024 21:36
TomAFrench added a commit that referenced this pull request Nov 13, 2024
* master:
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  feat: mock data for IVC (#9893)
  refactor: final token cleanup (#9864)
  chore: delete accidentally added file (#9912)
@benesjan benesjan mentioned this pull request Nov 13, 2024
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.

3 participants