chore!: do not use dynamic types in interfaces by Maddiaa0 · Pull Request #14203 · AztecProtocol/aztec-packages

Maddiaa0 · 2025-05-10T11:36:36Z

Overview

Rollup:

migrates propose interface to use the Header object directly, not bytes as will always know its type
re instantes the state commitment type rather than using bytes, as it is a fixed size object

Tests:

Load the header from the block snapshots directly, instead of working on the bytes object as we KNOW its type
- does this by shimming the alphabetical json into the real object in the load block section
- removes functions that set the header values based on bytes offsets, instead, just manipulate the object directly

Maddiaa0 · 2025-05-10T11:36:56Z

feat(contracts-periphery): coinbase and splits forwarders #14173
chore!: do not use dynamic types in interfaces #14203 👈 (View in Graphite)
next

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

just-mitch · 2025-05-12T10:57:34Z

l1-contracts/test/decoders/Decoders.t.sol

Just delete?

just-mitch

Beautiful.

LHerskind

Need a rebase for the conflicts but I like it 👍

LHerskind · 2025-06-04T07:55:03Z

l1-contracts/test/benchmark/happy.t.sol

-    header = DecoderBase.updateHeaderFeeRecipient(header, address(0));
-    header = DecoderBase.updateHeaderBaseFee(header, manaBaseFee);
-    header = DecoderBase.updateHeaderManaUsed(header, manaSpent);
+    header.lastArchiveRoot = archiveRoot;


Much better 🙏

LHerskind · 2025-06-04T07:56:54Z

yarn-project/archiver/src/archiver/data_retrieval.ts

  ];

-  const header = ProposedBlockHeader.fromBuffer(Buffer.from(hexToBytes(decodedArgs.header)));
+  // TODO(md): why is the proposed block header different to the actual block header?


Costs I believe. But I agree, foot gun territory

Yeah it's mainly for reducing cost - it's 316 bytes vs 592 bytes calldata and sha hashing. But if we prefer a more consistent type to reducing a little bit of cost, I can change it back!

LHerskind · 2025-06-04T07:58:21Z

yarn-project/end-to-end/src/composed/integration_l1_publisher.test.ts

          },
-          slotNumber: `0x${block.header.globalVariables.slotNumber.toBuffer().toString('hex').padStart(64, '0')}`,
-          timestamp: Number(block.header.globalVariables.timestamp.toBigInt()),
+          slotNumber: block.header.globalVariables.slotNumber.toBigInt(),


Would kinda have expected this to blow up. As I recall, we were doing the throwing around because json stringify were not too happy about the bigints 🤷

In process of rebasing and yes. This completely explodes all over the place.

LHerskind · 2025-06-04T08:01:41Z

yarn-project/stdlib/src/p2p/consensus_payload.ts

+      'uint8, ' + //domainSeperator
+        '(' +
+        'bytes32, ' + // archive
+        '((bytes32,uint32),((bytes32,uint32),(bytes32,uint32),(bytes32,uint32))), ' + // stateReference


Thats one ugly looking line 😬 Don't we have any neat way we could get this thing directly from the abis or so instead 🤔 Or guess it would just explode so maybe not too big a deal if easy to spot.

LHerskind · 2025-06-05T10:51:38Z

@Maddiaa0 can you take a look again at this? It seems like it was broken in #14466 because the data generation used the now changed defaults or so 🤷 @sirasistant running into some pain as it have been broken and it only popped up along with his changes.

LHerskind · 2025-06-06T13:43:36Z

Throwing this onto the queue again even if @charlielye call it a crime, as it seems like it failed to be merged, but no test failed 😅 so not sure why it did not make it.

Doing a rebase first though.

LHerskind · 2025-06-08T22:23:12Z

Latest failure on the merge queue was due to a sporadic issue in Anvil that Alex have dealt with in foundry-rs/foundry#10714

## Overview Rollup: - migrates propose interface to use the Header object directly, not bytes as will always know its type - re instantes the state commitment type rather than using bytes, as it is a fixed size object Tests: - Load the header from the block snapshots directly, instead of working on the bytes object as we KNOW its type - does this by shimming the alphabetical json into the real object in the load block section - removes functions that set the header values based on bytes offsets, instead, just manipulate the object directly --------- Co-authored-by: LHerskind <16536249+LHerskind@users.noreply.github.com>

Maddiaa0 requested review from LHerskind and just-mitch as code owners May 10, 2025 11:36

Maddiaa0 mentioned this pull request May 10, 2025

feat(contracts-periphery): coinbase and splits forwarders #14173

Closed

just-mitch reviewed May 12, 2025

View reviewed changes

l1-contracts/test/decoders/Decoders.t.sol Outdated

Copy link

Collaborator

just-mitch May 12, 2025

Choose a reason for hiding this comment

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

Just delete?

just-mitch approved these changes May 12, 2025

View reviewed changes

LHerskind approved these changes Jun 4, 2025

View reviewed changes

LHerskind changed the base branch from master to next June 5, 2025 10:43

LHerskind mentioned this pull request Jun 5, 2025

fix!: Make fee per gas uint128 across the protocol #14796

Merged

LHerskind changed the title ~~chore(contracts): do not use dynamic types in interfaces~~ chore!(contracts): do not use dynamic types in interfaces Jun 6, 2025

LHerskind changed the title ~~chore!(contracts): do not use dynamic types in interfaces~~ chore!: do not use dynamic types in interfaces Jun 6, 2025

LHerskind force-pushed the md/no-dynamic-types-in-interfaces branch 3 times, most recently from df9126f to 2370e77 Compare June 6, 2025 11:46

LHerskind added this pull request to the merge queue Jun 6, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 6, 2025

LHerskind force-pushed the md/no-dynamic-types-in-interfaces branch from 2370e77 to 2626607 Compare June 6, 2025 13:43

LHerskind added this pull request to the merge queue Jun 6, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 6, 2025

LHerskind force-pushed the md/no-dynamic-types-in-interfaces branch from 2626607 to be74542 Compare June 6, 2025 19:01

LHerskind self-assigned this Jun 8, 2025

Maddiaa0 added 6 commits June 8, 2025 22:23

feat: do not use dynamic types in propose interface

3d113ae

lint

978e92a

chore: delete decoders test

aa3d4a3

update ts

7d065a1

fix archiver test

45d07e6

fix: abi type has become more complicated

50b481c

Maddiaa0 and others added 3 commits June 8, 2025 22:25

lint

568d83b

chore: update test constants and fixtures

fa8abc7

chore: fix header hash + rename to proposed_header in sol

945751e

LHerskind force-pushed the md/no-dynamic-types-in-interfaces branch from be74542 to 945751e Compare June 8, 2025 22:36

LHerskind enabled auto-merge June 8, 2025 22:36

LHerskind added this pull request to the merge queue Jun 9, 2025

Merged via the queue into next with commit 403b1a8 Jun 9, 2025
5 checks passed

LHerskind deleted the md/no-dynamic-types-in-interfaces branch June 9, 2025 08:17

AztecBot mentioned this pull request Jun 9, 2025

chore(next): release 0.88.0 #14495

Closed

Conversation

Maddiaa0 commented May 10, 2025

Overview

Uh oh!

Maddiaa0 commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

just-mitch May 12, 2025

Choose a reason for hiding this comment

Uh oh!

just-mitch left a comment

Choose a reason for hiding this comment

Uh oh!

LHerskind left a comment

Choose a reason for hiding this comment

Uh oh!

LHerskind Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

LeilaWang Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LHerskind commented Jun 6, 2025

Uh oh!

Uh oh!

LHerskind commented Jun 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Maddiaa0 commented May 10, 2025 •

edited

Loading

LHerskind commented Jun 5, 2025 •

edited

Loading