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: Reintroduce Alloy #724

Merged
merged 27 commits into from Oct 2, 2023
Merged

Conversation

Evalir
Copy link
Contributor

@Evalir Evalir commented Sep 14, 2023

Rebases #617 to be up to date with the newest changes, mainly concerning bundlestate.

@Evalir Evalir force-pushed the reintroduce-alloy-rebased branch 2 times, most recently from dd6fdf2 to ff95074 Compare September 14, 2023 19:22
@Evalir Evalir marked this pull request as ready for review September 14, 2023 19:40
@gakonst
Copy link
Collaborator

gakonst commented Sep 15, 2023

You should be able to just git reset --hard origin/main and then git cherry-pick your commits on top of that to clean up your history, would really prefer we don't have all these things there ^^

@Evalir
Copy link
Contributor Author

Evalir commented Sep 15, 2023

oh yeah ik just hadn't done it—wanted to open the branch asap to leave the foundry drafts ready. Will do soon!

@rakita
Copy link
Member

rakita commented Sep 16, 2023

oh yeah ik just hadn't done it—wanted to open the branch asap to leave the foundry drafts ready. Will do soon!

If you have merged with origin/main (Have merge commit) it is easy to clean it up.
git reset --soft origin/main
You will be left only with changes and you then do
`git commit -m "Feat: Alloy integration"

@gakonst
Copy link
Collaborator

gakonst commented Sep 20, 2023

@Evalir failing CI blocking this

Running tests in ethtests/GeneralStateTests/...
Test "NoSrcAccount" (id: 0, path: ethtests/GeneralStateTests/stTransactionTest/NoSrcAccount.json) failed:
Test NoSrcAccount failed: state root mismatch: expected 0xa511d2fb090b111e2a9e1459c648060322ebb545c0a89a8edf5cebf11e4f1eb2, got 0x25298cb0779d10a5f411af0693043af3998891d7ee557d7fed4005b5e7ed690a

Traces:

Execution result: Err(
    Transaction(
        LackOfFundForMaxFee {
            fee: 21000,
            balance: 0x0_U256,
        },
    ),
)

Expected exception: Some("TR_NoFunds")

State before: CacheState {
    accounts: {
        0xd0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0: CacheAccount {
            account: Some(
                PlainAccount {
                    info: AccountInfo {
                        balance: 0x0_U256,
                        nonce: 0,
                        code_hash: 0xbc36789e7a1e281436464229828f817d6612f7b477d66[591](https://github.com/bluealloy/revm/actions/runs/6240889399/job/16941964718?pr=724#step:6:592)ff96a9e064bcc98a,
                        code: Some(
                            Bytecode {
                                bytecode: Bytes(0x00),
                                state: Raw,
                            },
                        ),
                    },
                    storage: {},
                },
            ),
            status: Loaded,
        },
    },
    contracts: {},
    has_state_clear: false,
}

State after: CacheState {
    accounts: {
        0xd0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0: CacheAccount {
            account: Some(
                PlainAccount {
                    info: AccountInfo {
                        balance: 0x0_U256,
                        nonce: 0,
                        code_hash: 0xbc36789e7a1e281436464229828f817d6612f7b477d66591ff96a9e064bcc98a,
                        code: Some(
                            Bytecode {
                                bytecode: Bytes(0x00),
                                state: Raw,
                            },
                        ),
                    },
                    storage: {},
                },
            ),
            status: Loaded,
        },
        0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b: CacheAccount {
            account: None,
            status: LoadedNotExisting,
        },
    },
    contracts: {},
    has_state_clear: false,
}

Environment: Env {
    cfg: CfgEnv {
        chain_id: 1,
        spec_id: FRONTIER,
        kzg_settings: Default,
        perf_analyse_created_bytecodes: Analyse,
        limit_contract_code_size: None,
        disable_coinbase_tip: false,
    },
    block: BlockEnv {
        number: 0x0000000000000000000000000000000000000000000000000000000000000001_U256,
        coinbase: 0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba,
        timestamp: 0x00000000000000000000000000000000000000000000000000000000000003e8_U256,
        gas_limit: 0x0000000000000000000000000000000000000000000000000000000005500000_U256,
        basefee: 0x000000000000000000000000000000000000000000000000000000000000000a_U256,
        difficulty: 0x0000000000000000000000000000000000000000000000000000000000020000_U256,
        prevrandao: Some(
            0x0000000000000000000000000000000000000000000000000000000000020000,
        ),
        excess_blob_gas: Some(
            0,
        ),
    },
    tx: TxEnv {
        caller: 0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b,
        gas_limit: 21000,
        gas_price: 0x0000000000000000000000000000000000000000000000000000000000000064_U256,
        transact_to: Call(
            0xd0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0d0,
        ),
        value: 0x0_U256,
        data: Bytes(0x),
        nonce: None,
        chain_id: None,
        access_list: [],
        gas_priority_fee: None,
        blob_hashes: [],
        max_fee_per_blob_gas: None,
    },
}
Finished execution. Total CPU time: 21.038529s
Error: Statetest(TestError { name: "NoSrcAccount", kind: StateRootMismatch { got: 0x25298cb0779d10a5f411af0693043af3998891d7ee557d7fed4005b5e7ed690a, expected: 0xa511d2fb090b111e2a9e1459c6480[603](https://github.com/bluealloy/revm/actions/runs/6240889399/job/16941964718?pr=724#step:6:604)22ebb545c0a89a8edf5cebf11e4f1eb2 } })

@Evalir
Copy link
Contributor Author

Evalir commented Sep 20, 2023

@gakonst we just need to rebase again—this test was also failing on main until today I see.

@gakonst
Copy link
Collaborator

gakonst commented Sep 20, 2023

very nice - defer to dragan otherwise

@Rjected
Copy link
Collaborator

Rjected commented Sep 28, 2023

mind rebasing this? If we merge paradigmxyz/reth#4737 in reth, then reth won't have this fix:
#757

@DaniPopes
Copy link
Collaborator

Opened Evalir#4

}

pub fn caller<H: Host>(interpreter: &mut Interpreter, _host: &mut H) {
gas!(interpreter, gas::BASE);
push_b256!(interpreter, B256::from(interpreter.contract.caller));
push_b256!(interpreter, interpreter.contract.caller.into_word());
Copy link
Member

Choose a reason for hiding this comment

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

I get the reference as in processor word (native type), but into_word feels strange to me. Bikeshedding here, nothing to be changed in the essence, maybe into_evm_word would be a more obvious name, but it does increase the fn name size.

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm 🎉

Including the Evalir#4. left a few nits (mostly like questions), Let us plan the merge on Monday (just to leave a few days for v3.4.0 to settle).

@rakita rakita merged commit af4146a into bluealloy:main Oct 2, 2023
8 checks passed
@github-actions github-actions bot mentioned this pull request Jan 12, 2024
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.

None yet

5 participants