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: token partial notes refactor pt. 2 - bridging #9600

Merged

Conversation

benesjan
Copy link
Contributor

Continuation of the token partial notes refactor now focused on token bridging.

Copy link
Contributor Author

benesjan commented Oct 30, 2024

@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from b5e9766 to 79870df Compare November 8, 2024 15:31
@benesjan benesjan changed the base branch from master to 10-31-refactor_nuking_pay_refund_with_shielded_rebate_flow November 8, 2024 15:31
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch 2 times, most recently from 9344ac0 to 5baeda8 Compare November 8, 2024 16:38
Base automatically changed from 10-31-refactor_nuking_pay_refund_with_shielded_rebate_flow to master November 8, 2024 16:57
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from 5baeda8 to c1a8e42 Compare November 8, 2024 16:58
@benesjan benesjan marked this pull request as ready for review November 8, 2024 17:57
@benesjan benesjan marked this pull request as draft November 8, 2024 18:32
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from 581ee9e to ef1b833 Compare November 8, 2024 19:01
@benesjan benesjan marked this pull request as ready for review November 8, 2024 19:17
@benesjan benesjan changed the title refactor: token partial notes reefactor pt. 2 - bridging refactor: token partial notes refactor pt. 2 - bridging Nov 8, 2024
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from ef1b833 to e5b74cb Compare November 8, 2024 23:09
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Changes to public function bytecode sizes

Generated at commit: 91bde1fc23d12021437299c3ef415d390353a235, compared to commit: b6216033b567e0d743e17c37754a20f9c893aa0e

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
TokenBridge::constructor +118 ❌ +3.65%
TokenBridge::public_dispatch -683 ✅ -3.02%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
TokenBridge::constructor 3,350 (+118) +3.65%
TokenBridge::public_dispatch 21,942 (-683) -3.02%

@benesjan benesjan added the e2e-all CI: Enables this CI job. label Nov 8, 2024
@benesjan benesjan force-pushed the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch from e5b74cb to c7e984b Compare November 9, 2024 03:43
@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
@@ -22,11 +22,7 @@ contract TokenPortal {
);

event DepositToAztecPrivate(
bytes32 secretHashForRedeemingMintedNotes,
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 ^ was the hash in the transparent note. It's no longer there (and neither is L2 address) meaning that only the message secret "owns" the bridged tokens.

@@ -27,15 +27,15 @@ contract TokenBridge {
// Storage structure, containing all storage, and specifying what slots they use.
#[storage]
struct Storage<Context> {
token: PublicMutable<AztecAddress, Context>,
token: SharedImmutable<AztecAddress, Context>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now need to read this in private as well so changed PublicMutable to Shared.

@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
#[private]
fn claim_private(
secret_hash_for_redeeming_minted_notes: Field, // secret hash used to redeem minted notes at a later time. This enables anyone to call this function and mint tokens to a user on their behalf
recipient: AztecAddress, // recipient of the bridged tokens
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are no longer redeeming "to" a TransparentNote and instead we just mint a real ValueNote to a recipient.

@@ -478,7 +478,7 @@ contract Token {
let token = Token::at(context.this_address());

// We prepare the transfer.
let hiding_point_slot = _prepare_transfer_to_private(to, &mut context, storage);
let hiding_point_slot = _prepare_transfer_to_private(from, to, &mut context, storage);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now pass the from param as it allows us to define the recipient of outgoing.

@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 requested a review from sklppy88 November 11, 2024 20:16
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.

LGTM 👍

@benesjan benesjan merged commit d513099 into master Nov 12, 2024
100 checks passed
@benesjan benesjan deleted the 10-30-refactor_token_partial_notes_reefactor_pt._2_-_bridging branch November 12, 2024 13:40
This was referenced Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-all CI: Enables this CI job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants