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

feat(en): Check recipient contract and function selector in consistency checker #1367

Merged

Conversation

slowli
Copy link
Contributor

@slowli slowli commented Mar 6, 2024

What ❔

  • Checks the recipient contract (should be equal to the main L1 contract address) and the Solidity function selector (should correspond to commitBlocks / commitBatches selector) in the consistency checker.
  • Reworks error handling in the consistency checker to be more maintainable.

Why ❔

Not checking these values allows to more-or-less easily fool the checker into accepting a commit transaction which is not really a commit transaction.

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.
  • Spellcheck has been run via zk spellcheck.
  • Linkcheck has been run via zk linkcheck.

@slowli slowli marked this pull request as ready for review March 6, 2024 13:09
@slowli slowli requested a review from perekopskiy March 8, 2024 12:54
Copy link
Collaborator

@RomanBrodetski RomanBrodetski left a comment

Choose a reason for hiding this comment

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

Very nice!

@slowli slowli added this pull request to the merge queue Mar 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 13, 2024
@slowli slowli added this pull request to the merge queue Mar 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 14, 2024
@slowli slowli added this pull request to the merge queue Mar 14, 2024
Merged via the queue into main with commit ea5c684 Mar 14, 2024
36 checks passed
@slowli slowli deleted the aov-pla-721-check-recipient-contract-and-function-selector-in branch March 14, 2024 09:46
github-merge-queue bot pushed a commit that referenced this pull request Mar 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[22.0.0](core-v21.1.0...core-v22.0.0)
(2024-03-21)


### ⚠ BREAKING CHANGES

* Use protocol version v22 as latest
([#1432](#1432))

### Features

* add docs for Dal
([#1273](#1273))
([66ceb0b](66ceb0b))
* adds debug_traceBlockByNumber.callFlatTracer
([#1413](#1413))
([d2a5e36](d2a5e36))
* **api:** introduce mempool cache
([#1460](#1460))
([c5d6c4b](c5d6c4b))
* **commitment-generator:** `events_queue` shadow mode
([#1138](#1138))
([9bb47fa](9bb47fa))
* **contract-verifier:** Allow sc code reverification
([#1455](#1455))
([5a37b42](5a37b42))
* **database:** add an optional master replica max connections settings
([#1472](#1472))
([e5c8127](e5c8127))
* **db:** Configurable maximum number of open RocksDB files
([#1401](#1401))
([b00c052](b00c052))
* **en:** Check recipient contract and function selector in consistency
checker ([#1367](#1367))
([ea5c684](ea5c684))
* Follow-up for DAL split
([#1464](#1464))
([c072288](c072288))
* **node_framework:** Ergonomic improvements
([#1453](#1453))
([09b6887](09b6887))
* **node_framework:** Only store each wiring layer once
([#1468](#1468))
([4a393dc](4a393dc))
* **node_framework:** Support for preconditions and oneshot tasks
([#1398](#1398))
([65ea881](65ea881))
* **node-framework:** Add eth sender layer
([#1390](#1390))
([0affdf8](0affdf8))
* **node-framework:** Add housekeeper layer
([#1409](#1409))
([702e739](702e739))
* Remove batch dry_run
([#1076](#1076))
([b82d093](b82d093))
* Separate Prover and Server DAL
([#1334](#1334))
([103a56b](103a56b))
* **storage:** Make the storage caches task cancellation aware
([#1430](#1430))
([ab532bb](ab532bb))
* support running consensus from snapshot (BFT-418)
([#1429](#1429))
([f9f4d38](f9f4d38))
* **tx-sender:** Limit concurrent tx submissions
([#1473](#1473))
([4bdf3ca](4bdf3ca))
* Use protocol version v22 as latest
([#1432](#1432))
([1757412](1757412))
* **vm:** Prestate tracer implementation
([#1306](#1306))
([c36be65](c36be65))


### Bug Fixes

* **core:** drop correct index of proof_generation_details table during
database migration
([#1199](#1199))
([76a6821](76a6821))
* **dal:** correct `tx_index_in_l1_batch` in
`get_vm_events_for_l1_batch`
([#1463](#1463))
([8bf37ac](8bf37ac))
* **node_framework:** Fix main node example
([#1470](#1470))
([ac4a744](ac4a744))
* **protocol:** Remove verifier address from protocol upgrade
([#1443](#1443))
([90dee73](90dee73))
* **reth:** use reth instead of geth
([#1410](#1410))
([bd98dc7](bd98dc7))
* **verified sources fetcher:** Use correct `contact_name` for
`SolSingleFile`
([#1433](#1433))
([0764227](0764227))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
montekki pushed a commit that referenced this pull request Mar 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[22.0.0](core-v21.1.0...core-v22.0.0)
(2024-03-21)


### ⚠ BREAKING CHANGES

* Use protocol version v22 as latest
([#1432](#1432))

### Features

* add docs for Dal
([#1273](#1273))
([66ceb0b](66ceb0b))
* adds debug_traceBlockByNumber.callFlatTracer
([#1413](#1413))
([d2a5e36](d2a5e36))
* **api:** introduce mempool cache
([#1460](#1460))
([c5d6c4b](c5d6c4b))
* **commitment-generator:** `events_queue` shadow mode
([#1138](#1138))
([9bb47fa](9bb47fa))
* **contract-verifier:** Allow sc code reverification
([#1455](#1455))
([5a37b42](5a37b42))
* **database:** add an optional master replica max connections settings
([#1472](#1472))
([e5c8127](e5c8127))
* **db:** Configurable maximum number of open RocksDB files
([#1401](#1401))
([b00c052](b00c052))
* **en:** Check recipient contract and function selector in consistency
checker ([#1367](#1367))
([ea5c684](ea5c684))
* Follow-up for DAL split
([#1464](#1464))
([c072288](c072288))
* **node_framework:** Ergonomic improvements
([#1453](#1453))
([09b6887](09b6887))
* **node_framework:** Only store each wiring layer once
([#1468](#1468))
([4a393dc](4a393dc))
* **node_framework:** Support for preconditions and oneshot tasks
([#1398](#1398))
([65ea881](65ea881))
* **node-framework:** Add eth sender layer
([#1390](#1390))
([0affdf8](0affdf8))
* **node-framework:** Add housekeeper layer
([#1409](#1409))
([702e739](702e739))
* Remove batch dry_run
([#1076](#1076))
([b82d093](b82d093))
* Separate Prover and Server DAL
([#1334](#1334))
([103a56b](103a56b))
* **storage:** Make the storage caches task cancellation aware
([#1430](#1430))
([ab532bb](ab532bb))
* support running consensus from snapshot (BFT-418)
([#1429](#1429))
([f9f4d38](f9f4d38))
* **tx-sender:** Limit concurrent tx submissions
([#1473](#1473))
([4bdf3ca](4bdf3ca))
* Use protocol version v22 as latest
([#1432](#1432))
([1757412](1757412))
* **vm:** Prestate tracer implementation
([#1306](#1306))
([c36be65](c36be65))


### Bug Fixes

* **core:** drop correct index of proof_generation_details table during
database migration
([#1199](#1199))
([76a6821](76a6821))
* **dal:** correct `tx_index_in_l1_batch` in
`get_vm_events_for_l1_batch`
([#1463](#1463))
([8bf37ac](8bf37ac))
* **node_framework:** Fix main node example
([#1470](#1470))
([ac4a744](ac4a744))
* **protocol:** Remove verifier address from protocol upgrade
([#1443](#1443))
([90dee73](90dee73))
* **reth:** use reth instead of geth
([#1410](#1410))
([bd98dc7](bd98dc7))
* **verified sources fetcher:** Use correct `contact_name` for
`SolSingleFile`
([#1433](#1433))
([0764227](0764227))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

4 participants