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

fix: Set attesters in Connection::adjust_genesis (BFT-489) #2429

Merged
merged 4 commits into from
Jul 11, 2024

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jul 11, 2024

What ❔

Sets the attesters field in Genesis when performing a re-genesis adjustment after the node starts and detects that its configuration has changed.

Testing

Main node only

To see if attestations work at all, I configured an attester key for the main node, and ran it with the following commands:

zk clean
zk init
zk server --components=api,tree,eth,state_keeper,housekeeper,commitment_generator,vm_runner_protective_reads,vm_runner_bwip,da_dispatcher,base_token_ratio_persister,consensus

Then I checked whether data was being added to the database:

psql -d postgres://postgres:notsecurepassword@localhost:5432/zksync_local 
psql (14.12 (Homebrew))
Type "help" for help.

zksync_local=# select count(1) from l1_batches_consensus;
 count 
-------
     2
(1 row)

zksync_local=# select * from l1_batches_consensus;
 l1_batch_number |                                                                                                                                                certificate                                                                                                                                                |         created_at         |         updated_at         
-----------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------------------------
               0 | {"msg": {"hash": {"keccak256": "FXT6d23sjaIHHl8g1xhAv8vYLCvKmtaGgO3+3eFxC8Q="}, "number": "0"}, "signatures": [{"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "1isW+az1GcoCDbjDhch9U5qakl6BMO033TWOQhdpAdFHV8MQbouVRqq3bTPUtROGYRVx1OkM61/BidaVhBS5JBw="}}]} | 2024-07-11 15:00:24.026841 | 2024-07-11 15:00:24.026841
               1 | {"msg": {"hash": {"keccak256": "L40KCOJ3fiy7B2V8CeXOjO/Vo/dKCs+EA9CFS9/dJLg="}, "number": "1"}, "signatures": [{"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "aP1Lw3u8BvGvWyjHauiEqhYvmidviG2jfdQdltnXSpUUQ4pOhKyT2FmGvplIMQ74vQDZaCbA12ap5WtHEJZEvRs="}}]} | 2024-07-11 15:00:34.069316 | 2024-07-11 15:00:34.069316
(2 rows)

Main and external node

I added attester keys to the external node config as well and gave it equal weights with the main node in the genesis spec, so they can only sign a QC together.

Started an external node with the following commands:

zk env ext-node
zk config compile
zk db setup
zk external-node -- --enable-consensus

And checked that the batch QC are inserted into the database, signed by both nodes:

psql -d postgres://postgres:notsecurepassword@localhost:5432/zksync_local -c "select * from l1_batches_consensus"
 l1_batch_number |                                                                                                                                                                                                                                             certificate                                                                                                                                                                                                                                             |         created_at         |         updated_at         
-----------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------------------------
               1 | {"msg": {"hash": {"keccak256": "erxoTxCmDETYR8ro8YeKD1AOZrZnc6e+4oYoR6MGPow="}, "number": "1"}, "signatures": [{"key": {"secp256k1": "AzCRTkGyJftvhRjVJ48MAUqYYQGMgFT5u0JbtlOFOPHJ"}, "sig": {"secp256k1": "1osJ9pPa8hB4BKNN6U9MDKommLOOKJ3hNOLpHdsqWRJN5dooK+CflKwQAIUtwjGa22EhmGulKv1fQs3stmSt3Rs="}}, {"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "xA8x0UcDq3ZultBn7ylM/G4+vRpsb+GQEbhUagAeZ2duvPRX5fOICR7z8k7wiJLine4/3abqcN/Uyn4FX97mpRw="}}]} | 2024-07-11 15:14:19.121081 | 2024-07-11 15:14:19.121081
(1 row)

It only signed the batch 1, not batch 0, because by the time I started the external node the main node already created two batches, and currently only the last vote counts. The certificate shows two items in signatures.

Why ❔

So that we can configure attesters (to sign L1 batches) on the stage 2 environment. Without this change the configured committee would be ignored.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@aakoshh aakoshh requested a review from brunoffranca July 11, 2024 12:07
@aakoshh aakoshh force-pushed the bft-489-add-attesters branch from 554f3eb to a28c651 Compare July 11, 2024 15:04
brunoffranca
brunoffranca previously approved these changes Jul 11, 2024
@brunoffranca brunoffranca enabled auto-merge July 11, 2024 15:06
@brunoffranca brunoffranca added this pull request to the merge queue Jul 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 11, 2024
@brunoffranca brunoffranca added this pull request to the merge queue Jul 11, 2024
Merged via the queue into main with commit ca4cb3c Jul 11, 2024
54 checks passed
@brunoffranca brunoffranca deleted the bft-489-add-attesters branch July 11, 2024 20:24
irnb pushed a commit to vianetwork/via-server that referenced this pull request Jul 12, 2024
…bs#2429)

## What ❔

Sets the `attesters` field in `Genesis` when performing a re-genesis
adjustment after the node starts and detects that its configuration has
changed.

### Testing


#### Main node only

To see if attestations work at all, I configured an attester key for the
main node, and ran it with the following commands:

```shell
zk clean
zk init
zk server --components=api,tree,eth,state_keeper,housekeeper,commitment_generator,vm_runner_protective_reads,vm_runner_bwip,da_dispatcher,base_token_ratio_persister,consensus
```

Then I checked whether data was being added to the database:

```console
❯ psql -d postgres://postgres:notsecurepassword@localhost:5432/zksync_local 
psql (14.12 (Homebrew))
Type "help" for help.

zksync_local=# select count(1) from l1_batches_consensus;
 count 
-------
     2
(1 row)

zksync_local=# select * from l1_batches_consensus;
 l1_batch_number |                                                                                                                                                certificate                                                                                                                                                |         created_at         |         updated_at         
-----------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------------------------
               0 | {"msg": {"hash": {"keccak256": "FXT6d23sjaIHHl8g1xhAv8vYLCvKmtaGgO3+3eFxC8Q="}, "number": "0"}, "signatures": [{"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "1isW+az1GcoCDbjDhch9U5qakl6BMO033TWOQhdpAdFHV8MQbouVRqq3bTPUtROGYRVx1OkM61/BidaVhBS5JBw="}}]} | 2024-07-11 15:00:24.026841 | 2024-07-11 15:00:24.026841
               1 | {"msg": {"hash": {"keccak256": "L40KCOJ3fiy7B2V8CeXOjO/Vo/dKCs+EA9CFS9/dJLg="}, "number": "1"}, "signatures": [{"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "aP1Lw3u8BvGvWyjHauiEqhYvmidviG2jfdQdltnXSpUUQ4pOhKyT2FmGvplIMQ74vQDZaCbA12ap5WtHEJZEvRs="}}]} | 2024-07-11 15:00:34.069316 | 2024-07-11 15:00:34.069316
(2 rows)
```

#### Main and external node

I added attester keys to the external node config as well and gave it
equal weights with the main node in the genesis spec, so they can only
sign a QC together.

Started an external node with the following commands:

```shell
zk env ext-node
zk config compile
zk db setup
zk external-node -- --enable-consensus
```

And checked that the batch QC are inserted into the database, signed by
both nodes:

```console
❯ psql -d postgres://postgres:notsecurepassword@localhost:5432/zksync_local -c "select * from l1_batches_consensus"
 l1_batch_number |                                                                                                                                                                                                                                             certificate                                                                                                                                                                                                                                             |         created_at         |         updated_at         
-----------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------+----------------------------
               1 | {"msg": {"hash": {"keccak256": "erxoTxCmDETYR8ro8YeKD1AOZrZnc6e+4oYoR6MGPow="}, "number": "1"}, "signatures": [{"key": {"secp256k1": "AzCRTkGyJftvhRjVJ48MAUqYYQGMgFT5u0JbtlOFOPHJ"}, "sig": {"secp256k1": "1osJ9pPa8hB4BKNN6U9MDKommLOOKJ3hNOLpHdsqWRJN5dooK+CflKwQAIUtwjGa22EhmGulKv1fQs3stmSt3Rs="}}, {"key": {"secp256k1": "A4snYq04KzUJC7QpkqP2RC1jhCXqVSjoAP/lqffUGFWJ"}, "sig": {"secp256k1": "xA8x0UcDq3ZultBn7ylM/G4+vRpsb+GQEbhUagAeZ2duvPRX5fOICR7z8k7wiJLine4/3abqcN/Uyn4FX97mpRw="}}]} | 2024-07-11 15:14:19.121081 | 2024-07-11 15:14:19.121081
(1 row)
```

It only signed the batch 1, not batch 0, because by the time I started
the external node the main node already created two batches, and
currently only the last vote counts. The certificate shows two items in
`signatures`.

## Why ❔

So that we can configure attesters (to sign L1 batches) on the stage 2
environment. Without this change the configured committee would be
ignored.

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.

---------

Co-authored-by: Bruno França <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jul 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.10.0](core-v24.9.0...core-v24.10.0)
(2024-07-22)


### Features

* Add blob size metrics
([#2411](#2411))
([41c535a](41c535a))
* **en:** Switch EN to use node framework
([#2427](#2427))
([0cee530](0cee530))
* **eth-sender:** add early return in sending new transactions to not
spam logs with errors
([#2425](#2425))
([192f2a3](192f2a3))
* **eth-watch:** Integrate decentralized upgrades
([#2401](#2401))
([5a48e10](5a48e10))
* L1 batch signing (BFT-474)
([#2414](#2414))
([ab699db](ab699db))
* **prover:** Make it possible to run prover out of GCP
([#2448](#2448))
([c9da549](c9da549))
* **zk_toolbox:** Small adjustment for zk toolbox
([#2424](#2424))
([ce43c42](ce43c42))


### Bug Fixes

* **eth-sender:** add bump of min 10% when resending txs to avoid
"replacement transaction underpriced"
([#2422](#2422))
([a7bcf5d](a7bcf5d))
* Set attesters in Connection::adjust_genesis (BFT-489)
([#2429](#2429))
([ca4cb3c](ca4cb3c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <[email protected]>
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