Introduction of P2P networking #666

PhilWindle · 2023-05-23T13:03:17Z

Description

This PR makes a number of changes all related to introducing basic P2P networking and multi-sequencer operation into the system. The primary changes are:

AztecNode is now an interface (which it probably should have always been). AztecNodeService is now the concrete implementation created by e2e tests.
There is now a rollup-provider package that creates a standalone instance of the AztecNodeService with a simple http interface. The client side to this http interface is HttpNode.
aztec-cli can be used to generate transactions and submit them to a running instance of rollup-provider as well as deploying the current set of rollup contracts to chain.
The sequencer no longer retries publishing a failed block. Instead it rolls back changes to it's world state and starts a new block. This is required for multi-sequencer operation where no orchestrated sequencer selection protocol is in place.
Introduction of an ethereum package containing configuration and common code for interaction with L1.
The L2Block calldata hash is published along with unverified data to chain. This enables the archiver to correctly identify which batch of unverified data corresponds with the successfully published L2Block in cases where a sequencer published unverified data but was unsuccessful in publishing a block.
The p2p package now has the ability to start P2P networking utilising LibP2P. Given the required configuration, the P2PClient will create a libp2p client and attempt to make contact with a bootstrap node. From there it uses Kademlia to discover other peers.
Once contact has been made with other peers their transaction pools are synchronised and further transactions are propagated through the network using a protocol id of '/aztec/tx/{rollup contract address}'
The messaging protocol is a partial implementation of Ethereum's transaction exchange but without the RLP message packing. We just use a simple binary serialisation for the time being.
There is a new package p2p-bootstrap which contains a standalone 'bootstrap' node service. This is used solely for seeding the peer tables of new joiners to the network. It doesn't take part in transaction gossiping.
A new end to end test that verifies the operation of the P2P network and successful transaction gossiping

Checklist:

I have reviewed my diff in github, line by line.
Every change is related to the PR description.
I have linked this pull request to the issue(s) that it resolves.
There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
The branch has been merged or rebased against the head of its merge target.
I'm happy for the PR to be merged at the reviewer's next convenience.

…ges into pw/big-branch

spalladino

Got up to yarn-project/aztec-node/src/aztec-node/http-node.ts, will continue with the rest on Monday. Looking awesome so far. I like that you also tackled the problems that arise from having to identify the source of unverified data due to having multiple sequencers.

spalladino · 2023-05-24T23:26:04Z

.circleci/config.yml

      - noir-contracts: *yarn_project
      - noir-compiler: *yarn_project
      - sequencer-client: *yarn_project
      - types: *yarn_project
      - circuits-js: *yarn_project
+      - rollup-provider: *yarn_project

      - e2e-join:


Add the new jobs to e2e-join requirements, so all unit tests finish before we start the e2e ones

Missed that, thanks

spalladino · 2023-05-24T23:31:11Z

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

-      sender: EthAddress.random().toString(),
+      l2BlockNum: BigInt(l2Block.number),
+      l2BlockHash: `0x${l2Block.getCalldataHash().toString('hex')}`,
+      sender: EthAddress.random(),


Suggested change

sender: EthAddress.random(),

sender: EthAddress.random().toString(),

This should let you remove the nasty as unknown in the cast. See #648.

Nice, done.

spalladino · 2023-05-24T23:31:43Z

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

-    transactionHash: `0x${l2BlockNum}`,
-  } as Log<bigint, number, undefined, typeof UnverifiedDataEmitterAbi, 'ContractDeployment'>;
+    transactionHash: `0x${l2Block.number}`,
+  } as unknown as Log<bigint, number, undefined, typeof UnverifiedDataEmitterAbi, 'ContractDeployment'>;


Try removing here as well

spalladino · 2023-05-24T23:33:53Z

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

@@ -37,5 +49,7 @@ export function getConfigEnvVars(): ArchiverConfig {
    unverifiedDataEmitterContract: UNVERIFIED_DATA_EMITTER_ADDRESS
      ? EthAddress.fromString(UNVERIFIED_DATA_EMITTER_ADDRESS)
      : EthAddress.ZERO,
+    searchStartBlock: SEARCH_START_BLOCK ? +SEARCH_START_BLOCK : 0,
+    apiKey: API_KEY || '',


I'd avoid using empty strings to signal empty values. Let's change apiKey to be string | undefined if it could be unset.

spalladino · 2023-05-24T23:36:27Z

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

@@ -123,20 +130,28 @@ export class Archiver implements L2BlockSource, UnverifiedDataSource, ContractDa
      this.nextL2BlockFromBlock,
      nextExpectedRollupId,
    );
+
+    // create the block number -> block hash mapping to ensure we retrieve the appropriate events
+    const blockHashMapping: { [key: string]: Buffer } = {};


Suggested change

const blockHashMapping: { [key: string]: Buffer } = {};

const blockHashMapping: { [key: string]: Buffer | undefined } = {};

So the compiler forces you to check that a given key is in the mapping before trying to use it

spalladino · 2023-05-24T23:39:19Z

yarn-project/aztec-cli/contracts/zk_token_contract.json

Let's import the artifact from noir-contracts instead of copying it here manually

Ah yeah, that change is already done. Removed the legacy file.

spalladino · 2023-05-24T23:41:22Z

yarn-project/aztec-cli/src/deploy_l1_contracts.ts

Is this copied from end-to-end/src/deploy_l1_contracts.ts? If so, let's delete it from there.

It is. In Aztec Connect we had a blockchain package which handled everything. Environment config, contract wrappers, chain scanning and event parsing, publishing, contract deployment scripts etc. In Aztec 3 everything is scattered. So the archiver has code for block scanning, sequencer-client has the block publishing etc. I'm wondering if we want to re-instate blockchain as a place for common chain operations/config. I could move environment into it.

Done, moved contract deployment code to common blockchain package.

spalladino · 2023-05-24T23:46:37Z

yarn-project/aztec-node/src/aztec-node/http-node.ts

+export function txToJson(tx: Tx) {
+  return {
+    data: tx.data?.toBuffer().toString('hex'),
+    unverified: tx.unverifiedData?.toBuffer().toString('hex'),
+    txRequest: tx.txRequest?.toBuffer().toString('hex'),
+    proof: tx.proof?.toBuffer().toString('hex'),
+    newContractPublicFunctions: tx.newContractPublicFunctions?.map(f => f.toBuffer().toString('hex')) ?? [],
+    enqueuedPublicFunctions: tx.enqueuedPublicFunctionCalls?.map(f => f.toBuffer().toString('hex')) ?? [],
+  };
+}


You can pass in a custom serializer object rather than manually converting every buffer to string. See the implementation of toFriendlyJSON for an example.

And aside from that, why not use the serialization methods we already have, and push everything over the wire as binary data?

I just used strings for ease of debugging. Being able to get legible output by cURLing the endpoints is pretty useful. We probably want to change this for at least some endpoints for a production system.

spalladino · 2023-05-24T23:47:11Z

yarn-project/aztec-node/src/aztec-node/http-node.ts

+ * @param json - The JSON representation of the transaction.
+ * @returns The deserialised transaction.
+ */
+export function txFromJson(json: any) {


Same here, you can pass a reviver to JSON.parse to make your life easier.

yarn-project/aztec-node/src/aztec-node/http-node.ts

PhilWindle · 2023-05-25T09:33:34Z

yarn-project/p2p/src/service/tx_messages.ts

+  return message.subarray(4);
+}
+
+/**


I created these serialisation functions as we didn't have anything for Tx objects. Could possibly just become toBuffer and fromBuffer on Tx. Would prefer to use the existing serialisation but we would need to handle undefined properties.

but we would need to handle undefined properties.

We could probably introduce a Maybe or Optional wrapper that just writes down an initial byte to indicate if the value is set or not, similar to how the Vector class writes the size first.

However, since we're moving to msgpack now, maybe it makes sense to do things the msgpack way instead? We may want to loop in @ludamad here.

yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts

Maddiaa0

I gave this a power read but everything looks great, nothing much to add ontop of santaigos points / just got a couple of nits, (maybe something around the deepcopy, seems we could make a generic function for that)

yarn-project/end-to-end/scripts/run_tests

yarn-project/environment/src/aztec_chain.ts

yarn-project/sequencer-client/src/publisher/config.ts

yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts

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

yarn-project/aztec-node/src/aztec-node/http-node.ts

yarn-project/aztec-node/src/aztec-node/server.ts

yarn-project/blockchain/src/aztec_chain.ts

yarn-project/blockchain/package.json

yarn-project/world-state/src/world-state-db/index.ts

ludamad

LGTM at a powerglance

spalladino

Hell of a PR! 🚀

spalladino · 2023-05-29T13:42:13Z

yarn-project/end-to-end/src/e2e_p2p_network.test.ts

+    // shutdown all nodes.
+    for (const context of contexts) {
+      await context.node.stop();
+      await context.rpcServer.stop();
+    }
+    await bootstrapNode.stop();


Consider moving to an afterEach block, so it runs even if the test fails

spalladino · 2023-05-29T13:42:44Z

yarn-project/end-to-end/src/e2e_p2p_network.test.ts

+    // now ensure that all txs were successfully mined
+    for (const context of contexts) {


Does it make sense to also assert that the current "view" of the world state is the same for all nodes?

spalladino · 2023-05-29T13:44:27Z

yarn-project/end-to-end/scripts/docker-compose-p2p.yml

+  p2p-bootstrap:
+    image: aztecprotocol/p2p-bootstrap:latest
+    ports:
+      - '40400:40400'
+    command: 'start'
+    environment:
+      DEBUG: 'aztec:*'
+      P2P_TCP_LISTEN_PORT: 40400
+      P2P_TCP_LISTEN_IP: '0.0.0.0'
+      P2P_ANNOUNCE_HOSTNAME: 'p2p-bootstrap'
+      PEER_ID_PRIVATE_KEY: '0a260024080112205ea53185db2e52dae74d0d4d6cadc494174810d0a713cd09b0ac517c38bc781e1224080112205ea53185db2e52dae74d0d4d6cadc494174810d0a713cd09b0ac517c38bc781e1a44080112402df8b977f356c6e34fa021c9647973234dff4df706c185794405aafb556723cf5ea53185db2e52dae74d0d4d6cadc494174810d0a713cd09b0ac517c38bc781e'


Is this used? I saw a createBootstrap in the e2e test that hints that a different bootstrap node is created in the same process as the test.

This isn't currently used. I had a vision of running the bootstrap node and the test as different containers but the build system isn't setup to publish the images. Everything has to run inside of e2e. I left this here in case I get time to take another look.

spalladino · 2023-05-29T13:45:48Z

yarn-project/ethereum/src/index.ts

+  return {
+    chainInfo: foundry,
+    rpcUrl,
+  } as EthereumChain;


Avoid as, since it hides type errors. It's better to set the expected return type in the function declaration.

spalladino · 2023-05-29T13:46:14Z

yarn-project/ethereum/src/testnet.ts

+  return {
+    chainInfo: chain,
+    rpcUrl: chain.rpcUrls.default.http[0],
+  } as EthereumChain;


spalladino · 2023-05-29T14:35:59Z

yarn-project/p2p/src/service/tx_messages.ts

+  return message.subarray(4);
+}
+
+/**


but we would need to handle undefined properties.

We could probably introduce a Maybe or Optional wrapper that just writes down an initial byte to indicate if the value is set or not, similar to how the Vector class writes the size first.

However, since we're moving to msgpack now, maybe it makes sense to do things the msgpack way instead? We may want to loop in @ludamad here.

spalladino · 2023-05-29T14:37:56Z

yarn-project/rollup-provider/src/app.ts

+    const postData = JSON.parse((await stream.readAll()) as string);
+    const tx = txFromJson(postData);


We'll need to put in some validation here, to make sure we're not passing in garbage within a tx

spalladino · 2023-05-29T14:38:28Z

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

@@ -35,7 +38,8 @@ export function getConfigEnvVars(): SequencerClientConfig {
    unverifiedDataEmitterContract: UNVERIFIED_DATA_EMITTER_ADDRESS
      ? EthAddress.fromString(UNVERIFIED_DATA_EMITTER_ADDRESS)
      : EthAddress.ZERO,
-    publisherPrivateKey: Buffer.from(SEQ_PUBLISHER_PRIVATE_KEY || ''),
+    publisherPrivateKey: Buffer.from(SEQ_PUBLISHER_PRIVATE_KEY || '', 'hex'),


spalladino · 2023-05-29T14:39:51Z

yarn-project/sequencer-client/src/publisher/l1-publisher.ts

@@ -272,7 +289,7 @@ export class L1Publisher implements L2BlockReceiver {
      try {
        return await this.txSender.getTransactionReceipt(txHash);
      } catch (err) {
-        this.log(`Error getting tx receipt`, err);
+        //this.log(`Error getting tx receipt`, err);


Is this for silencing those annoying logs in the e2e tests? If so, can we differentiate between a "receipt not yet available" and an actual error?

Hmm, can't remember now. I've uncommented it for now.

spalladino · 2023-05-29T14:46:39Z

yarn-project/types/src/tx.ts

+   * @param tx - The transaction to be cloned.
+   * @returns The cloned transaction.
+   */
+  static clone(tx: Tx): Tx {


I think cloneDeep could handle this automatically

PhilWindle and others added 30 commits April 30, 2023 20:21

WIP

bc8ffbb

WIP

ae8ea14

WIP

0d8d282

WIP

1d4c691

WIP

f397798

WIP

c81db00

WIP

fc7408e

Merge branch 'master' into pw/big-branch

9eae85c

WIP

cb6efb9

Merge branch 'master' into pw/big-branch

d91e373

Merge fixes

71b3573

Working multi provider setup

bd809b4

Increase height of roots trees

36ae237

WIP

1267dbb

Formatting

5c287a7

WIP

1d295fd

Reverted change

b3caf10

WIP

fdd8c6b

Docker fix

23c2d22

WIP

3974ab2

WIP

c1c2e6b

Setup relay nodes

e323258

Add dotenv to rollup provider

fc5715b

Don't propaget txs to node that we know have already seen them

eaa4ebb

Merge branch 'pw/big-branch' of github.com:AztecProtocol/aztec3-packa…

1edb73e

…ges into pw/big-branch

Allow specification of client or server as P2P node

2b0cc43

Add dotenv for bootstrap node

3b9ca32

Add env vars for announce ip:port and enabling nat

290362f

Sync transaction pools on node start

348df7b

Handle node not started

db9c9b2

PhilWindle added 2 commits May 24, 2023 22:13

Merge fix

57852af

More merge fixes

e2651e1

PhilWindle marked this pull request as ready for review May 24, 2023 22:22

PhilWindle added 5 commits May 24, 2023 22:26

Merge fixes

05f4ae6

More fixes

8d2d3eb

More fixes

f2174a6

yarn prepare

37c8090

Build fix

1debaca

PhilWindle changed the title ~~Pw/big branch~~ Introduction of P2P networking May 24, 2023

Test fixes

a753c2b

spalladino reviewed May 24, 2023

View reviewed changes

PhilWindle commented May 25, 2023

View reviewed changes

rahul-kothari reviewed May 25, 2023

View reviewed changes

yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts Outdated Show resolved Hide resolved

PhilWindle added 6 commits May 25, 2023 09:39

Review fixes

5a4c96e

Allow configuration of min/max peer counts

99d1284

Review fixes

6b09faf

Review changes

bd62b9f

Build fixes

9eed093

Formatting

a728e83

Maddiaa0 reviewed May 25, 2023

View reviewed changes

PhilWindle added 5 commits May 25, 2023 13:12

Review changes

d7eafce

Build fix

43cbfea

Reverted old commit

34cbee4

Merge branch 'master' into pw/big-branch

d2fdbd4

Updated snapshot

b21d6cc

ludamad approved these changes May 25, 2023

View reviewed changes

PhilWindle merged commit f919dd4 into master May 25, 2023

PhilWindle deleted the pw/big-branch branch May 25, 2023 16:13

Maddiaa0 mentioned this pull request May 25, 2023

Make it easy to configure chain-id #614

Closed

spalladino reviewed May 29, 2023

View reviewed changes

	sender: EthAddress.random(),
	sender: EthAddress.random().toString(),

	const blockHashMapping: { [key: string]: Buffer } = {};
	const blockHashMapping: { [key: string]: Buffer \| undefined } = {};

		// now ensure that all txs were successfully mined
		for (const context of contexts) {

		const postData = JSON.parse((await stream.readAll()) as string);
		const tx = txFromJson(postData);

Introduction of P2P networking #666

Introduction of P2P networking #666

Conversation

PhilWindle commented May 23, 2023 • edited Loading

Description

Checklist:

spalladino 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

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

Choose a reason for hiding this comment

ludamad left a comment

Choose a reason for hiding this comment

spalladino 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

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

PhilWindle commented May 23, 2023 •

edited

Loading