feat: `#[derive(Packable)]` #11531

benesjan · 2025-01-27T16:55:21Z

In this PR I implement auto-derivation of Packable trait.

It uses the same underlying functionality as derivation of Serialize but it calls the generate_serialize_to_fields and generate_deserialize_from_fields with packing enabled. This means that if some of the struct members has the Packable trait implemented it gets used instead of the intrinsic Noir serialization.

benesjan · 2025-01-27T16:55:55Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

benesjan · 2025-01-27T20:32:16Z

noir-projects/aztec-nr/address-note/src/address_note.nr

+        address::AztecAddress,
+        constants::GENERATOR_INDEX__NOTE_NULLIFIER,
+        hash::poseidon2_hash_with_separator,
+        traits::{Packable, Serialize},


All the files with notes now generally need to import Packable as because of the change in noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr.

See comment in that file for details.

As mentioned in the serialize pr, it'd be good for derive to not require extra imports from the caller. We can either autoimport, or just use fully qualified paths in the derived impl.

benesjan · 2025-01-27T20:33:35Z

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -1,38 +1,9 @@
-use dep::aztec::{macros::aztec, protocol_types::traits::{Deserialize, Packable, Serialize}};
-
-// I tried using #[derive(Serialize, Deserialize)] macro here but for whatever reason it fails to compile.


Moved the note to a separate file because then the derivation works.

I find it quite weird to have the note defined in the same file as the contract.

Do we find this bug worthy of trying to fix?

Yes, definitely

https://aztecprotocol.slack.com/archives/C04QF64EDNV/p1738170160977319?thread_ts=1738097938.001229&cid=C04QF64EDNV

benesjan · 2025-01-27T20:39:43Z

noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr

@@ -104,8 +104,8 @@ pub comptime fn generate_deserialize_from_fields<N>(
            }
            let packed_fields = packed_fields_quotes.join(quote {,});

-            // No we call unpack on the type
-            result = quote { aztec::protocol_types::traits::Packable::unpack([ $packed_fields ]) };


This is the reason why the Packable needs to be imported now.

Having the full path here was a mistake since the dep could be called differently than aztec and this might also be used inside the protocol circuits where there is no aztec dep.

We probably could figure the correct import at comptime but would do that in a separate PR if we find this desirable.

Ahh, I see. Can't we have the full path to proto_types::traits so that it'd work both here, in aztec, and in the user's code?

You would then need to figure out whether you are in the types crate or not as in the types crate you would not use dep::aztec::protocol_types but just crate.

Jake promised me he will send a PR such that I can quote a Type directly and get a correct path. This would then make this straighforward. Until that is done I've created this issue to track it.

benesjan · 2025-01-27T20:40:53Z

noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr

+            quote {_}
+        } else {
+            token
+        };


This was a bug and it got triggered by me using Serialize derivation in noir-projects/noir-contracts/contracts/card_game_contract/src/game.nr

github-actions · 2025-01-27T21:20:30Z

Changes to public function bytecode sizes

Generated at commit: 5deef6e40843f91aa45792ec78426ab6c5804ddb, compared to commit: 0cfd0fe72fb52dcb00eec26294b7d434a673992a

🧾 Summary (100% most significant diffs)

Program	Bytecode size in bytes (+/-)	%
CardGame::on_card_played	+1,277 ❌	+24.82%
CardGame::on_game_joined	+976 ❌	+21.19%
CardGame::start_game	+976 ❌	+16.23%
CardGame::on_cards_claimed	+970 ❌	+14.45%
Lending::get_asset	+228 ❌	+11.41%
CardGame::public_dispatch	+1,301 ❌	+9.52%
Lending::init	+101 ❌	+3.84%
Lending::get_position	+57 ❌	+1.34%
Lending::update_accumulator	+63 ❌	+1.15%
Lending::public_dispatch	+8 ❌	+0.03%

Full diff report 👇

Program	Bytecode size in bytes (+/-)	%
CardGame::on_card_played	6,422 (+1,277)	+24.82%
CardGame::on_game_joined	5,583 (+976)	+21.19%
CardGame::start_game	6,990 (+976)	+16.23%
CardGame::on_cards_claimed	7,681 (+970)	+14.45%
Lending::get_asset	2,227 (+228)	+11.41%
CardGame::public_dispatch	14,967 (+1,301)	+9.52%
Lending::init	2,728 (+101)	+3.84%
Lending::get_position	4,310 (+57)	+1.34%
Lending::update_accumulator	5,538 (+63)	+1.15%
Lending::public_dispatch	25,988 (+8)	+0.03%

sklppy88

Looks great, have some tiny qns but will approve to unblock ! 🙏

noir-projects/noir-contracts/contracts/lending_contract/src/asset.nr

noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr

noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr

benesjan commented Jan 27, 2025

View reviewed changes

benesjan marked this pull request as ready for review January 27, 2025 20:42

benesjan changed the title ~~feat: #[derive(Packable)]~~ feat: #[derive(Packable)] Jan 27, 2025

benesjan mentioned this pull request Jan 27, 2025

feat: using #[derive(Serialize)] more #11541

Merged

benesjan requested review from nventuro and sklppy88 January 27, 2025 21:58

benesjan mentioned this pull request Jan 27, 2025

feat: packing events #11544

Closed

benesjan force-pushed the 01-16-refactor_using_uintnote_in_crowdfunding_and_claim branch from 31ce3f7 to 787e700 Compare January 28, 2025 17:20

benesjan force-pushed the 01-27-feat_derive_packable_ branch from 9f5f05a to 12261b5 Compare January 28, 2025 17:20

benesjan force-pushed the 01-16-refactor_using_uintnote_in_crowdfunding_and_claim branch from 787e700 to f7e3d93 Compare January 28, 2025 18:29

benesjan force-pushed the 01-27-feat_derive_packable_ branch from 12261b5 to 7b76054 Compare January 28, 2025 18:29

sklppy88 approved these changes Jan 29, 2025

View reviewed changes

benesjan marked this pull request as draft January 29, 2025 13:48

benesjan force-pushed the 01-16-refactor_using_uintnote_in_crowdfunding_and_claim branch from f7e3d93 to 612e2e8 Compare January 29, 2025 13:50

benesjan force-pushed the 01-27-feat_derive_packable_ branch from 7b76054 to 0883f93 Compare January 29, 2025 13:50

Base automatically changed from 01-16-refactor_using_uintnote_in_crowdfunding_and_claim to master January 29, 2025 14:44

benesjan added 5 commits January 29, 2025 14:45

feat: #[derive(Packable)]

1d9fd3d

importing Packable because all notes now require it

6c1ced2

deriving packable

fca3ea0

WIP

75360c1

better docs

41a9f7d

benesjan added 4 commits January 29, 2025 14:45

improved browser test instructions

c0ff5bf

re-introducing comment

7ba5469

fixed comment

2ef42ca

reverting incorrect change + adding comments

3800e92

benesjan force-pushed the 01-27-feat_derive_packable_ branch from 44c5bb7 to 3800e92 Compare January 29, 2025 14:45

benesjan marked this pull request as ready for review January 29, 2025 14:45

benesjan enabled auto-merge (squash) January 29, 2025 14:46

benesjan merged commit 83fe192 into master Jan 29, 2025
70 of 71 checks passed

benesjan deleted the 01-27-feat_derive_packable_ branch January 29, 2025 15:29

AztecBot mentioned this pull request Jan 29, 2025

chore(master): Release 0.73.0 #11496

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `#[derive(Packable)]` #11531

feat: `#[derive(Packable)]` #11531

benesjan commented Jan 27, 2025 •

edited

Loading

benesjan commented Jan 27, 2025 •

edited

Loading

benesjan Jan 27, 2025

nventuro Jan 29, 2025

benesjan Jan 29, 2025

benesjan Jan 27, 2025

nventuro Jan 29, 2025

benesjan Jan 29, 2025

benesjan Jan 27, 2025

nventuro Jan 29, 2025

benesjan Jan 29, 2025 •

edited

Loading

benesjan Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading

sklppy88 left a comment •

edited

Loading

		@@ -1,38 +1,9 @@
		use dep::aztec::{macros::aztec, protocol_types::traits::{Deserialize, Packable, Serialize}};

		// I tried using #[derive(Serialize, Deserialize)] macro here but for whatever reason it fails to compile.

feat: #[derive(Packable)] #11531

feat: #[derive(Packable)] #11531

Conversation

benesjan commented Jan 27, 2025 • edited Loading

benesjan commented Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benesjan Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

benesjan Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 27, 2025 • edited Loading

Changes to public function bytecode sizes

🧾 Summary (100% most significant diffs)

sklppy88 left a comment • edited Loading

Choose a reason for hiding this comment

feat: `#[derive(Packable)]` #11531

feat: `#[derive(Packable)]` #11531

benesjan commented Jan 27, 2025 •

edited

Loading

benesjan commented Jan 27, 2025 •

edited

Loading

benesjan Jan 29, 2025 •

edited

Loading

benesjan Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading

sklppy88 left a comment •

edited

Loading