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

feat: nullable note fields info in ABI #8901

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented Oct 1, 2024

To be able to robustly position publicly delivered fields in the serialized note array when processing notes we need to know the name of each field, index of where the field starts in the serialized note array and whether the field is nullable/partial.

image

Copy link
Contributor Author

benesjan commented Oct 1, 2024

@benesjan benesjan marked this pull request as ready for review October 1, 2024 11:56
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Changes to public function bytecode sizes

Generated at commit: 884a1617e38ce8ff343d2203f6bb96621267bb20, compared to commit: c68a5eff1f438860f2aa85d59c48ba9f85fc3d0d

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
Lending::public_dispatch +988 ❌ +1.29%
TokenBridge::public_dispatch +14 ❌ +0.04%
Uniswap::public_dispatch +14 ❌ +0.04%
TokenBlacklist::public_dispatch +28 ❌ +0.02%
Token::public_dispatch +7 ❌ +0.01%
Auth::public_dispatch -7 ✅ -0.02%
AvmTest::public_dispatch -21 ✅ -0.02%
FPC::public_dispatch -14 ✅ -0.12%
NFT::public_dispatch -70 ✅ -0.17%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
Lending::public_dispatch 77,650 (+988) +1.29%
TokenBridge::public_dispatch 32,353 (+14) +0.04%
Uniswap::public_dispatch 33,748 (+14) +0.04%
TokenBlacklist::public_dispatch 141,193 (+28) +0.02%
Token::public_dispatch 67,253 (+7) +0.01%
Auth::public_dispatch 33,604 (-7) -0.02%
AvmTest::public_dispatch 94,599 (-21) -0.02%
FPC::public_dispatch 11,393 (-14) -0.12%
NFT::public_dispatch 40,338 (-70) -0.17%

@@ -15,6 +15,10 @@ use dispatch::generate_public_dispatch;

/// Marks a contract as an Aztec contract, generating the interfaces for its functions and notes, as well as injecting
/// the `compute_note_hash_and_optionally_a_nullifier` function PXE requires in order to validate notes.
///
/// Note: This is a module annotation, so the returned quote gets injected inside the module (contract) itself.
/// The difference between a module and a regular struct or functionn annotation is that module annotations inject
Copy link
Contributor

Choose a reason for hiding this comment

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

The first line of the note is fine, the second one I feel like it's repeating noir docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nuked that in 352f8a1

@@ -144,14 +148,37 @@ comptime fn generate_note_properties(s: StructDefinition) -> Quoted {
}
}

pub(crate) comptime fn generate_note_export(s: StructDefinition, note_type_id: Field) -> Quoted {
/// Generates note export for a given note struct. The export is a global variable that contains note type id,
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@Thunkar Thunkar 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 force-pushed the 10-01-feat_nullable_note_fields_info_in_abi branch from 6ce30c5 to 352f8a1 Compare October 1, 2024 13:24
@benesjan benesjan enabled auto-merge (squash) October 1, 2024 13:27
@benesjan benesjan merged commit e0d5e06 into master Oct 1, 2024
51 checks passed
@benesjan benesjan deleted the 10-01-feat_nullable_note_fields_info_in_abi branch October 1, 2024 13:55
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