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(cli): check user provided genesis if not first run #1278

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

yangby-cryptape
Copy link
Collaborator

@yangby-cryptape yangby-cryptape commented Jul 19, 2023

What this PR does / why we need it:

This PR add a feature that:

And this PR fixes a potential issue:

  • If it's not the first run, the genesis in Axon struct is incorrect.

    • Because the state_root and receipts_root will be set to the correct values only when it is the first run.

      • They are set in the method execute_transactions(..):

        axon/core/run/src/lib.rs

        Lines 206 to 207 in 1f34617

        self.genesis.block.header.state_root = self.state_root;
        self.genesis.block.header.receipts_root = resp.receipt_root;

      • But if it's not first run, execute_transactions(..) will not be called.

        axon/core/run/src/lib.rs

        Lines 127 to 131 in 1f34617

        match storage.get_latest_block(Context::new()).await {
        Ok(_) => {
        log::info!("The Genesis block has been initialized.");
        return Ok(());
        }

        self.execute_transactions(&mut mpt, &trie_db, &storage)

    This is not a bug currently, because these fields are not used.

Which issue(s) this PR fixes:

Fixes #1275.

Which toolchain this PR adaption:

No Breaking Change

Special notes for your reviewer:

Since it has to run a temporary empty chain to generate the genesis state, so it uses temporary directories to create RocksAdapter and RocksTrieDB.

But system_contract::init(..) sets a global environment variable, so it has to be called with the real path.

I am not sure whether it OK or not:

CI Description

CI Name Description
Chaos CI Test the liveness and robustness of Axon under terrible network condition
Cargo Clippy Run cargo clippy --all --all-targets --all-features
Coverage Test Get the unit test coverage report
E2E Test Run end-to-end test to check interfaces
Code Format Run cargo +nightly fmt --all -- --check and cargo sort -gwc
Web3 Compatible Test Test the Web3 compatibility of Axon
v3 Core Test Run the compatibility tests provided by Uniswap V3
OCT 1-5 | 6-10 | 11 | 12-15 | 16-19 Run the compatibility tests provided by OpenZeppelin

CI Usage

Check the CI you want to run below, and then comment /run-ci.

CI Switch

  • Chaos CI
  • Cargo Clippy
  • Coverage Test
  • E2E Tests
  • Code Format
  • Unit Tests
  • Web3 Compatible Tests
  • OCT 1-5 And 12-15
  • OCT 6-10
  • OCT 11
  • OCT 16-19
  • v3 Core Tests

@yangby-cryptape

This comment was marked as off-topic.

1 similar comment
@yangby-cryptape

This comment was marked as off-topic.

@yangby-cryptape yangby-cryptape force-pushed the yangby/feature/check-genesis-state branch from 71978d5 to 7e65518 Compare July 19, 2023 12:30
@yangby-cryptape yangby-cryptape marked this pull request as ready for review July 19, 2023 12:36
@yangby-cryptape yangby-cryptape requested a review from a team as a code owner July 19, 2023 12:36
@Flouse Flouse requested review from driftluo and KaoImin and removed request for ashuralyk July 19, 2023 12:41
@Flouse

This comment was marked as outdated.

@Flouse

This comment was marked as off-topic.

@axon-bot

This comment was marked as outdated.

@yangby-cryptape

This comment was marked as off-topic.

@axon-bot

This comment was marked as outdated.

@yangby-cryptape yangby-cryptape force-pushed the yangby/feature/check-genesis-state branch from 7e65518 to cb109ce Compare July 20, 2023 02:46
@yangby-cryptape

This comment was marked as off-topic.

@axon-bot

This comment was marked as outdated.

core/run/src/lib.rs Outdated Show resolved Hide resolved
@KaoImin KaoImin changed the title feat(cli): check user provided genesis if not first run feat(cli)!: check user provided genesis if not first run Jul 20, 2023
@yangby-cryptape yangby-cryptape force-pushed the yangby/feature/check-genesis-state branch from cb109ce to 4c287a6 Compare August 2, 2023 09:18
@yangby-cryptape

This comment was marked as off-topic.

@axon-bot

This comment was marked as outdated.

@yangby-cryptape yangby-cryptape changed the title feat(cli)!: check user provided genesis if not first run feat(cli): check user provided genesis if not first run Aug 2, 2023
@yangby-cryptape yangby-cryptape force-pushed the yangby/feature/check-genesis-state branch from 4c287a6 to 1d52850 Compare August 2, 2023 09:59
@yangby-cryptape

This comment was marked as off-topic.

@axon-bot

This comment was marked as outdated.

@yangby-cryptape yangby-cryptape requested a review from KaoImin August 3, 2023 02:13
@yangby-cryptape

This comment was marked as off-topic.

@yangby-cryptape yangby-cryptape added this pull request to the merge queue Aug 3, 2023
Merged via the queue into main with commit 8f3eded Aug 3, 2023
@yangby-cryptape yangby-cryptape deleted the yangby/feature/check-genesis-state branch August 3, 2023 13:39
@Flouse Flouse added t:feature and removed feature labels Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The executable binary still can run normally even the genesis state is changed.
4 participants