refactor(blocks tree): remove historic roots trees #1355

Maddiaa0 · 2023-08-01T18:43:30Z

Description

closes: #1163

Now the blocks tree baseline is added, we can remove the old historic blocks trees

This Pr:

Deletes the historic roots trees for private data, contracts and l1 to l2 messages
Removes all checks from smart contract / rollups, in a previous pr the block hash check was added
Add a way for rpc server to recalculate the previous block hash. (this is in progress)
Add getBlock (singular, it was annoying getting an array when i only need one)

…ree roots

LHerskind

Minor stuff, mainly around a typo in a name and to create issues for some todos. Otherwise good to me.

LHerskind · 2023-08-07T20:58:48Z

circuits/cpp/src/aztec3/circuits/abis/constant_historic_block_data.hpp

    using fr = typename NCT::fr;
    using boolean = typename NCT::boolean;

+    // Private data
    fr private_data_tree_root = 0;
    fr nullifier_tree_root = 0;
    fr contract_tree_root = 0;
    fr l1_to_l2_messages_tree_root = 0;
    fr blocks_tree_root = 0;
    fr private_kernel_vk_tree_root = 0;  // TODO: future enhancement


@dbanks12 or @iAmMichaelConnor I guess the todo here is one of yours? Can one of you create an issue for it?

LHerskind · 2023-08-07T21:09:37Z

yarn-project/aztec-rpc/src/simulator_oracle/index.ts

      privateKernelVkTreeRoot: Fr.ZERO,
+      // TODO: work out how to get the previous globals hash in here


Is there an issue for this?

LHerskind · 2023-08-07T21:16:25Z

yarn-project/sequencer-client/src/sequencer/utils.ts

 import { computeGlobalsHash } from '@aztec/circuits.js/abis';
 import { MerkleTreeOperations } from '@aztec/world-state';

 /**
 * Fetches the private, nullifier, contract tree and l1 to l2 messages tree roots from a given db and assembles a CombinedHistoricTreeRoots object.
 */
-export async function getConstantBlockHashData(
+export async function getconstantHistoricBlockData(


Should probably be getConstantHistoricBlockData

LHerskind · 2023-08-07T21:17:04Z

yarn-project/sequencer-client/src/sequencer/public_processor.ts

@@ -86,7 +86,7 @@ export class PublicProcessor {
    protected publicProver: PublicProver,
    protected contractDataSource: ContractDataSource,
    protected globalVariables: GlobalVariables,
-    protected blockHashData: ConstantBlockHashData,
+    protected blockHashData: ConstantHistoricBlockData,


Maybe better to rename to not refer to hash in some names but not others?

LHerskind · 2023-08-07T21:18:01Z

yarn-project/sequencer-client/src/simulator/public_executor.ts

      privateKernelVkTreeRoot: Fr.ZERO,
+
+      // TODO: work out how to get the previous globals hash in here
+      prevGlobalVariablesHash: Fr.ZERO,


As earlier, should this have an issue?

turns out im both cases we no longer needed this method as i had injected it in higher up. Have delet

rahul-kothari · 2023-08-08T15:15:47Z

circuits/cpp/src/aztec3/constants.hpp

@@ -223,7 +220,7 @@ constexpr size_t MAX_NOTES_PER_PAGE = 10;
 constexpr size_t VIEW_NOTE_ORACLE_RETURN_LENGTH = MAX_NOTES_PER_PAGE * (MAX_NOTE_FIELDS_LENGTH + 1) + 2;

 constexpr size_t CALL_CONTEXT_LENGTH = 6;
-constexpr size_t COMMITMENT_TREES_ROOTS_LENGTH = 5;
+constexpr size_t BLOCK_DATA_LENGTH = 7;


can you write a comment here so that if people add any more trees to the block tree, they can easily find this and remember to update it?

rahul-kothari · 2023-08-08T15:18:41Z

yarn-project/acir-simulator/src/client/simulator.ts

@@ -166,7 +165,7 @@ export class AcirSimulator {
        abi,
        AztecAddress.ZERO,
        EthAddress.ZERO,
-        PrivateHistoricTreeRoots.empty(),
+        ConstantHistoricBlockData.empty(),


Why is this a better name that something like HistoricBlockDataTreeRoots (like its predecessor)?

It's no longer just roots, as the global variables are included within it. Finding the right name for it has been difficult, Ive flipped it around a few times. But im open to changing it again as its important to get right.
@iAmMichaelConnor do you have any suggestions?

Hmm, I suppose this can be thought of as a block header cant it?

LHerskind

If you add the issues as pointed out, looks good to me.

LHerskind · 2023-08-08T15:28:17Z

yarn-project/noir-libs/noir-aztec/src/abi.nr

@@ -296,7 +296,7 @@ struct PublicCircuitPublicInputs {
    new_l2_to_l1_msgs: [Field; crate::abi::MAX_NEW_L2_TO_L1_MSGS_PER_CALL],
    unencrypted_logs_hash: [Field; NUM_FIELDS_PER_SHA256],
    unencrypted_log_preimages_length: Field,
-    block_hash_data: BlockHashData, // TODO: This is not present in cpp or ts, do we need to include it?
+    block_data: ConstantBlockData, // TODO: This is not present in cpp or ts, do we need to include it?


Is this still a todo? If so can you add issue for it.

No longer a todo, however i will change the name to match exactly

LHerskind · 2023-08-08T15:30:48Z

yarn-project/noir-libs/noir-aztec/src/abi.nr

-        // We do not include block_hash_data since it's not in the cpp hash
-        // inputs.push(self.block_hash_data.hash());
+        // We do not include block_data since it's not in the cpp hash
+        // inputs.push(self.block_data.hash());


Can you add an issue for this, should be handled instead of just living as comment in here.

Maddiaa0 added 28 commits July 26, 2023 16:42

feat: add blocks tree data to l2 block

8553fa0

feat: use constants to represent offsets

4580093

chore: forge fmt

1991a57

fix: linter

e0808b8

fix: stub circuit output for the meantime

98692dd

feat: add naive block hash calculations to root rollup circuit

2942f95

Merge branch 'master' into md/blocks-tree-circuits

e8f9fbc

temp

376e71a

Merge branch 'master' into md/blocks-tree-circuits

9403830

fix: update snapshot

7520217

feat: include block hash calcs in sequencer

3c9422c

fix: make calculate block hash a cbind

de68edd

Merge branch 'master' into md/blocks-tree-circuits

520b9a6

feat: add block tree checking in ts

55fbd5f

clean

f1d9fc7

lint

3ae5a4d

review comments

011a150

Merge branch 'master' into md/blocks-tree-circuits

830721f

fix: include generator indices

035c41f

lint

5340a79

update snapshot

d9bda7b

fix: remove references to block num in factories

455058a

Merge branch 'master' into md/blocks-tree-circuits

515bb60

feat: read trees against historic block root

03cc42e

feat: remove other checks from the base rollup

3670fac

feat: update ts and snapshot

e0129f8

feat: remove old tree checks in base rollup

acd2413

fix

2bc675f

Base automatically changed from md/blocks-tree-circuits to master August 2, 2023 14:29

fix: get previous globals in transactions

1b8be8c

Maddiaa0 requested review from LHerskind and rahul-kothari August 4, 2023 18:38

Maddiaa0 added 12 commits August 4, 2023 18:43

lint so good, hmmm, lint so gooood

ce5a6be

refactor: processor && fix public tests

b3fcccc

fix: serialise

f065629

fix: missed test updates

8034c95

fix: reinstate circuits test

f8ce11e

Merge branch 'master' into md/blocks-tree-use-blocks-tree

50ff634

fix: recompile aztec js account contracts

cb3f80c

refactor: rename historic block hash data and remove nested private t…

a681273

…ree roots

refactor: rename in ts

ae967ae

cleanup

4a0f15a

Merge branch 'master' into md/blocks-tree-use-blocks-tree

862a553

chore: update snapshot

c0c5526

LHerskind reviewed Aug 7, 2023

View reviewed changes

Maddiaa0 added 6 commits August 8, 2023 14:07

minor rename

1f102a5

review fixes

02d4e85

Merge branch 'master' into md/blocks-tree-use-blocks-tree

8468b2e

fmt

deefe17

chore(fmt): formatting 2; electric boogaloo

9210bf9

Merge branch 'master' into md/blocks-tree-use-blocks-tree

5882121

rahul-kothari reviewed Aug 8, 2023

View reviewed changes

LHerskind approved these changes Aug 8, 2023

View reviewed changes

Maddiaa0 added 2 commits August 9, 2023 09:55

fix: final review comments

2cfa1eb

Merge branch 'master' into md/blocks-tree-use-blocks-tree

f4f22e3

Maddiaa0 enabled auto-merge (squash) August 9, 2023 10:00

merge fix

81dd1f3

Maddiaa0 merged commit ac935e1 into master Aug 9, 2023
1 check passed

Maddiaa0 deleted the md/blocks-tree-use-blocks-tree branch August 9, 2023 10:35

Maddiaa0 mentioned this pull request Aug 11, 2023

feat: store recent public data tree / globals hash of most recent block in rpc server #1537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(blocks tree): remove historic roots trees #1355

refactor(blocks tree): remove historic roots trees #1355

Maddiaa0 commented Aug 1, 2023 •

edited

Loading

LHerskind left a comment

LHerskind Aug 7, 2023

LHerskind Aug 7, 2023

LHerskind Aug 7, 2023

LHerskind Aug 7, 2023

LHerskind Aug 7, 2023

Maddiaa0 Aug 8, 2023

rahul-kothari Aug 8, 2023

rahul-kothari Aug 8, 2023

Maddiaa0 Aug 8, 2023 •

edited

Loading

Maddiaa0 Aug 9, 2023

LHerskind left a comment

LHerskind Aug 8, 2023

Maddiaa0 Aug 9, 2023

LHerskind Aug 8, 2023

Maddiaa0 Aug 9, 2023

		privateKernelVkTreeRoot: Fr.ZERO,
		// TODO: work out how to get the previous globals hash in here

refactor(blocks tree): remove historic roots trees #1355

refactor(blocks tree): remove historic roots trees #1355

Conversation

Maddiaa0 commented Aug 1, 2023 • edited Loading

Description

LHerskind left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maddiaa0 Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LHerskind left a 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

Maddiaa0 commented Aug 1, 2023 •

edited

Loading

Maddiaa0 Aug 8, 2023 •

edited

Loading