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: Run cdk-erigon with cdk config files transparently for the user #88

Merged
merged 24 commits into from
Oct 8, 2024

Conversation

vcastellm
Copy link
Contributor

@vcastellm vcastellm commented Sep 20, 2024

Description

Using the Rust CLI it's now possible to run cdk-erigon that it's present in system's path in a single step providing only the node config file and the genesis file.

cdk --config ../kurtosis-cdk/genesis/cdk-node-config.toml --chain ../kurtosis-cdk/genesis/genesis.json erigon

Avoiding the necessity of using multiple configuration files and different configuration for cdk-node and cdk-erigon

image

NOTE: This Rust code is the first iteration into the CLI and can be much improved and refactored in subsequent PRs


Steps to test this in local:

  1. Requirements: Run your kurtosis environment and have a cdk-erigon version installed in your system's PATH
  2. scripts/local_config
  3. cargo run -- --config ./tmp/cdk/local_config/test.kurtosis.toml --chain ./tmp/cdk/local_config/genesis.json erigon

@vcastellm vcastellm requested a review from a team September 25, 2024 08:04
Render the config files in tmp and run erigon passing the path of the
config file.
@vcastellm vcastellm marked this pull request as ready for review September 25, 2024 08:08
Copy link
Contributor

@joanestebanr joanestebanr left a comment

Choose a reason for hiding this comment

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

I don't see how to build the rust client, maybe it needs an entry on Makefile?

@vcastellm
Copy link
Contributor Author

vcastellm commented Sep 25, 2024

I don't see how to build the rust client, maybe it needs an entry on Makefile?

It's quite straightforward, in Rust there's the concept of "build scripts" arbitrary commands that can be executed at build time, in this case we use this one https://github.com/0xPolygon/cdk/blob/main/crates/cdk/build.rs, it builds cdk-node at build time so both binaries are built and run with just:

cargo run

Or just build with:

cargo build

Better entry on README


Updated the readme with new instructions using make

config/example-config.toml Outdated Show resolved Hide resolved
config/example-config.toml Outdated Show resolved Hide resolved
crates/cdk-config/src/aggregator.rs Outdated Show resolved Hide resolved
crates/cdk/src/main.rs Show resolved Hide resolved
crates/cdk/src/config_render.rs Outdated Show resolved Hide resolved
crates/cdk/src/config_render.rs Outdated Show resolved Hide resolved
@vcastellm vcastellm requested a review from a team October 2, 2024 13:30
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Dockerfile Fixed Show fixed Hide fixed
Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

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

Generally speaking, should we match the current configuration of the CDK, as we are most likely missing some fields (EthTxManager for example)?

README.md Show resolved Hide resolved
crates/cdk-config/src/aggregator.rs Show resolved Hide resolved
crates/cdk-config/src/sequence_sender.rs Show resolved Hide resolved
crates/cdk-config/src/telemetry.rs Outdated Show resolved Hide resolved
@@ -41,7 +42,7 @@
# CONTAINER FOR RUNNING BINARY
FROM --platform=${BUILDPLATFORM} debian:bookworm-slim

RUN apt-get update && apt-get install -y ca-certificates postgresql-client
RUN apt-get update && apt-get install -y ca-certificates postgresql-client libssl-dev && rm -rf /var/lib/apt/lists/*

Check notice

Code scanning / SonarCloud

Arguments in long RUN instructions should be sorted

<!--SONAR_ISSUE_KEY:AZJnfBrSpFFsRD9OkOZM-->Sort these package names alphanumerically. <p>See more on <a href="https://sonarcloud.io/project/issues?id=0xPolygon_cdk&issues=AZJnfBrSpFFsRD9OkOZM&open=AZJnfBrSpFFsRD9OkOZM&pullRequest=88">SonarCloud</a></p>
@vcastellm
Copy link
Contributor Author

Generally speaking, should we match the current configuration of the CDK, as we are most likely missing some fields (EthTxManager for example)?

Good observation @Stefan-Ethernal in general we're not adding all config values just some at this point, some of them we use it to construct the config, others are simply missing and others are there to use them for status checking in the future. I preferred just leaving them as is instead of removing them, we can refactor them in upcoming PRs in case the usage we're going to give them.

Copy link

sonarqubecloud bot commented Oct 7, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@vcastellm vcastellm merged commit edabc4c into develop Oct 8, 2024
9 of 11 checks passed
@vcastellm vcastellm deleted the cli-genesis branch October 8, 2024 16:35
vcastellm added a commit that referenced this pull request Oct 17, 2024
* test: Added check for EOA transfer E2E Test (#75)

* tests: PoC bridge testing with bats

* Run bats in e2e

* Remove unused make lines

* tests: wip

* test: better

* Let's see

* test: fix test

* test: use cdk image

* test: bats path

* test: fix

* test: deposit on 1

* test: wait for claim

* test: timeout

* test: timeout

* test: increase timeout

* test: apply feedback

* ci: lint action

* test: do not prepare if already present

* test: Send EOA and deploy contract E2E tests using Bats (#69)

* feat: add helper functions for contract deployment and sending transactions using cast commands

* feat: send EOA transaction test basic

* feat: add CDK_ERIGON_NODE_NAME var

* feat: invoke _common_setup function

* feat: deploy ERC20Mock contract E2E test

* feat: more strict assertions and use run instead of $

* feat: tweaks

* fix: change the way transaction hash is extracted

* fix: change the way transactionHash gets fetched from the output

* fix: cast call helper function and invocation of balanceOf function

* test: use RAW_PRIVATE_KEY env variable for sender private key

* fix: address feedback from @vcastellm

* fix: Linters warning fixes (#74)

* feat: use the latest golangci-lint version and fix config warnings

* fix: linter warnings

* fix: linter warnings (part 2)

* fix: propagate the error from aggregator.Start

* fix: format golangci config file

* fix: suppress gosec overflow issues

* fix: exclude G115 gosec linter rule

* fix: use crypto/rand number generator

* test: apply feedback

* test: lint

* ci: increase lint timeout

* test: balance check for ether transafers

* test: wip

* fix: apply feedback

* fix: cast commands

* test: add edge case

* fix: tests

* fix: resolve conflicts

* fix: tests

* fix: tests

* fix: rpc

* fix: typo

* fix: checkTransactionSuccess

* fix: send eoa transaction test

* refactor: feedback

* refactor: feedback

* fix: simplifications

* fix: even more simplifications

---------

Co-authored-by: Victor Castell <0x@vcastellm.xyz>
Co-authored-by: Stefan Negovanović <93934272+Stefan-Ethernal@users.noreply.github.com>
Co-authored-by: Stefan Negovanović <stefan@ethernal.tech>

* fix: Aggregator: use sequenceBatch maxTimestamp as TimestampLimit  (#84)

- Update to zkevm-synchronizer-l1 v1.0.1
- Check unexpected/deprecateds fields on config file

* First L1 Info tree in which bridge is included (#70)

* wip

* wip

* WIP

* decoding direct and indeirecr assets and messages works

* connect everything

* fix building contract scripts

* fix building contract scripts

* wip

* WIP

* tree migrated to SQLite

* wip

* wip

* bridgesync working with sqlite

* pass tests

* minor cleaning

* add GetBlockByLER func

* handle err not found

* merge develop

* use memory for sqlite on the tests

* increase timestamp to pass UT

* review

* finished implementation

* replace l1bridge2infosync for rpc logic

* ut wip

* unit tests for new rpc funcs

* add UTs for new methods in the l1infotreesync processor

* fix UTs

* pass linter

* add PR requests

* add missing processor calls

* fixx linter

* rephrase binnary search formula

* rephrase binnary search formula

* feat: add missing methods needed to generate PP certificate (#82)

* wip

* wip

* WIP

* decoding direct and indeirecr assets and messages works

* connect everything

* fix building contract scripts

* fix building contract scripts

* wip

* WIP

* tree migrated to SQLite

* wip

* wip

* bridgesync working with sqlite

* pass tests

* minor cleaning

* add GetBlockByLER func

* handle err not found

* merge develop

* use memory for sqlite on the tests

* increase timestamp to pass UT

* review

* finished implementation

* replace l1bridge2infosync for rpc logic

* ut wip

* unit tests for new rpc funcs

* add UTs for new methods in the l1infotreesync processor

* fix UTs

* pass linter

* add PR requests

* add missing processor calls

* fixx linter

* feat: add missing methods needed to generate PP certificate

* fix linter

* feat: seq sender sanity check l1infotree (#86)

* test: Custom native gas token transfer E2E test (#79)

* test: Add smart contract e2e tests (#85)

* feat: sc tests

* fix: tests

* fix: test

* feat: more tests

* fix: tests

* fix: tests

* fix: seprate keys for contracts deployement

* fix: rename sender private key env var

* fix: failing e2e tests

* chore: rename functions ( bash style)

* fix: access lists e2e tests

* refactor: apply feedback

* fix: polycli version

* fix: polycli workflow

* refactor: apply feedback

---------

Co-authored-by: Stefan Negovanović <stefan@ethernal.tech>

* feat: Add e2e reorg tests to syncers  (#56)

* feat: Added e2e tests to the syncer

* fix: UTs

* fix: comments

* fix: comments 2

* fix: rebase

* fix: lint

* fix: rebase remove old test

* fix: another rebase fix

* fix: stress test

* fix: ut build

* fix: rebuild tree after reorg

* fix: comments

---------

Co-authored-by: Goran Rojovic <goran.rojovic@ethernal.tech>

* ci: update polycli version (#89)

* feat: update relase regex (#93)

* Revert "feat: update relase regex (#93)" (#94)

This reverts commit e9a1ec0.

* ci: pin kurtosis-cdk version in tests (#92)

* add more tests claimtest (#96)

* feat: Use InitL1InfoRootMap (#87)

- Fix l1infotreesync. Error UNIQUE constraint failed: rollup_exit_root.hash on VerifyBatches Event
- l1infotreesync: Add verification for contract address. The problem is that if you set bad address cdk run normally but you don't get any information about L1InfoTree.
- l1infotreesync: Add support to `InitL1InfoRootMap`
- Allow to use `InitL1InfoRootMap` if there are no leaves on L1InfoTree

Internal:
- Fix local config file generation for debug on vscode (`./scripts/local_config`)
    - Add support to `contractVersions`
    - Remove param `-custom-network-file` that is no longer used
- Refactor `l1infotreesync/processor.go` in multiples files
- Change some tree functions to use a `tx db.Querier` instead of `ctx context.Context`: context was not used to do DB query was using `db` directly. In some test I need to query over current tx

* feat: New `zkevm-ethtx-manager` version (#98)

* fix: update zkevm-ethtx-manager version

* fix: UTs

* feat: remove data stream from sequence sender (#63)

* feat: implement batch interface

* feat: update sync lib

* feat: enable test

* feat: update config files

* feat: fix test

* feat: fix test

* feat: batch string method

* feat: update kurtosis commit

* fix: comments

* fix: comments

* fix: linter

* fix: vars

* fix: error return

* fix: regex

* fix: hex to to bytes

* feat: update kurtosis template

* feat: use config-file on CDK for kurtosis e2e tests (#99)

- The e2e test for CDK use a local configuration template instead of the one on kurtosis branch: to allow to develop changes on config file is more easy if we are able to use the config file on CDK repo. After that we can update config file and cdk image on kurtosis repo
- The local configuration for run on vscode use the same template (`test/config/kurtosis-cdk-node-config.toml.template`)  as the e2e test
- Fix error on CI `test-e2e.yml` with the duplicated `ref` key

* feat: update zkevm-synchronizer-l1 to v1.0.2 (#102)

-  update zkevm-synchronizer-l1 from [v1.0.1](https://github.com/0xPolygonHermez/zkevm-synchronizer-l1/releases/tag/v1.0.1) to [v1.0.2](https://github.com/0xPolygonHermez/zkevm-synchronizer-l1/releases/tag/v1.0.2): 
       - fix: [#119](0xPolygonHermez/zkevm-synchronizer-l1#119), fails if there are multiple sequencedBatch in same bock because a SQL have wrong order by 
       - feat: add check to DB configuration
       - fix: downgrade migrations sql lite, remove scheme prefix from tables names

* refactor: simplify running Kurtosis combinations (#101)

Using a set of pre-defined combinations of components,
we're going to test the versions we're interested in.

* feat: Initial for packagers (#90)

* Initial for packagers
* Adding BUILD_SCRIPT_DISABLED=1 to debian packagers

* feat: Integrate ethtxmanager with sql lite storage (#97)

* chore: fix comments

* feat: integrate the v0.2.0 eth tx manager spec

* fix: rename DBPath to StoragePath in the ethtxmanager config

* chore: rename according to golang guidelines

* chore: deprecate the PersistenceFilename config param

* test: unit test for obsoleted PersistenceFIlename configuration parameter

* feat: adapt kurtosis cdk and fix errors (#104)

- Some request of ports return the protocol and another no so the code have been updated to support port with protocol or without (`http://127.0.0.1:8023` or `127.0.0.1:8023`) 
- Also, to support changing end-point names have include a list of 'end-points' to get the URL / port
- There was a hardcoded path in config file that have been changed by vars and left the absolute path used by kurtosis as deault value

* feat: Sequence sender unit tests (#103)

* feat: Proof Cleaning (#105)

* feat: proof cleaning

* feat: improve log

* feat: Upgraded cdk-rpc dep (#108)

* fix: cdk-523, update zkevm-synchronizer-l1 to v1.0.3 (#109)

* ci: bump kurtosis tag and cdk-erigon (#106)

* ci: bump cdk-erigon

Also remove testing fork9-rollup

* fix: check ssender config (#110)

* fix: check ssender config

* feat: add default value

* fix: if sanity check of l1infotreeUpdateV2 fails, means an error and need to stop syncing (#113)

* feat: Run cdk-erigon with cdk config files transparently for the user (#88)

* feat: dynamically generate erigon config

Render the config files in tmp and run erigon passing the path of the
config file.

* feat: add node components param
* docs: update readme
* refactor: remove admin address
* fix: adapt dockerfile
* refactor: tune makefile

* ci: fix release tags (#114)

* ci: fix release tags

* ci: fix regex for release

* refactor: accept empty config file (#118)

This is necessary because we're not going to check for any values at this point.

* refactor: set default on all config values (#119)

* test: add test for aggregator  (#100)

* test: mockery setuo

* feat: make interface exported

* test: add aggregator e2e test

* test: wip

* test: wip

* fix: tests

* fix: tests

* test: wip

* test: refactored

* fix: Makefile

* fix: lint

* fix: test

* fix: race condition

* fix: apply feedback

* fix: apply feedback

* fix: apply feedback and remove cyclic dependency

* fix: remove comments

---------

Co-authored-by: Toni Ramírez <toni@polygon.technology>

* feat: add versions command (#120)

* refactor: not all commands need the same params
* feat: versions command

* feat: Add support for all the contracts on `test/helpers` so it's easy to build E2E tests (#115)

* fix: shorthand and add descriptions (#123)

* fix: shorthand and add descriptions
* ci: bump cdk-erigon

* feat: L1 Info Tree sync testing (#124)

* feat: warning on agglayer rate limit (#122)

* feat: retry on agglayer rate limit exceeded

* chore: bump erigon (#127)

* feat: currentStreamBatchRaw sanity check (#131) (#132)

* fix: Fixing L1 info tree sync tests (#130) (#134)

* fix: Fix l1infotreesync tests

Co-authored-by: rbpol <rbehma@polygon.technology>

* ci: bump kurtosis version (#128)

* ci: bump kurtosis version

Also bumb necessary versions in combinations.

* test: compute gas-price
* test: remove txspammer

---------

Co-authored-by: Rachit Sonthalia <54906134+rachit77@users.noreply.github.com>
Co-authored-by: Stefan Negovanović <93934272+Stefan-Ethernal@users.noreply.github.com>
Co-authored-by: Stefan Negovanović <stefan@ethernal.tech>
Co-authored-by: Joan Esteban <129153821+joanestebanr@users.noreply.github.com>
Co-authored-by: Arnau Bennassar <arnaubennassar5@gmail.com>
Co-authored-by: rbpol <rbehma@polygon.technology>
Co-authored-by: Goran Rojovic <goran.rojovic@ethernal.tech>
Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>
Co-authored-by: laisolizq <37299818+laisolizq@users.noreply.github.com>
Co-authored-by: Goran Rojovic <100121253+goran-ethernal@users.noreply.github.com>
Co-authored-by: Daniel Jones <105369507+djpolygon@users.noreply.github.com>
Co-authored-by: Toni Ramírez <toni@polygon.technology>
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