bedrock: make fee recipient be SequencerFeeVault

[tynes]

Description

The fee recipient or block.coinbase is now set to a hardcoded value of the predeploy SequencerFeeVault. This reduces configuration that is consensus criticial.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: bd651e5165edb0230507bd2d072aeb98f2d76b32

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mergify[bot]]

Hey @tynes! This PR has merge conflicts. Please fix them before continuing review.

[mergify[bot]]

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

[mslipper]

Seeing this in the devnet + yarn monorepo tests:

tasks/rollup-config.ts(47,11): error TS2741: Property 'fee_recipient_address' is missing in type '{ genesis: { l1: { hash: string; number: number; }; l2: { hash: string; number: number; }; l2_time: number; }; block_time: any; max_sequencer_drift: any; seq_window_size: any; channel_timeout: any; l1_chain_id: any; ... 4 more ...; deposit_contract_address: string; }' but required in type 'OpNodeConfig'.

The fees test in op-e2e is failing as a result of this as well. LGTM once these are fixed.

[tynes]

=== CONT  TestFees
    system_test.go:1003: 
        	Error Trace:	/root/project/op-e2e/system_test.go:1003
        	Error:      	Not equal: 
        	            	expected: 32816
        	            	actual  : 210000
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -3,3 +3,3 @@
        	            	  abs: (big.nat) (len=1) {
        	            	-  (big.Word) 32816
        	            	+  (big.Word) 210000
        	            	  }
        	Test:       	TestFees
        	Messages:   	l1 fee mismatch

[tynes]

Not sure why the devnet with deployed contracts is breaking here, will need to run locally to debug

[trianglesphere]

we should instantiate different fee vaults for each type of L2 fee. I think we are sending it to EOAs in bedrock now? So instead of predeploys.SequencerFeeVault, we can have L2FeeRecipient, L1FeeRecipient, BaseFeeRecipient or something like that.

We decide a while ago that we shouldn't split it into three streams, but two (basefee + L1 fee) & (the miner tip). https://www.notion.so/oplabs/L2-Fee-Payout-8b3485ce8e61475ca31bfc38994a5b06. The addresses are supposed to be contracts rather than EOAs, but they were normal contracts rather than pre-deploys.

[tynes]

LGTM, but in a follow-up PR we should instantiate different fee vaults for each type of L2 fee. I think we are sending it to EOAs in bedrock now? So instead of predeploys.SequencerFeeVault, we can have L2FeeRecipient, L1FeeRecipient, BaseFeeRecipient or something like that.

I have a PR that i'm almost ready to push that does this, will have it up soon

[tynes]

Unclear to me why this is failing, need to investigate more:

timeout 5m npx hardhat deposit-erc20 --network devnetL1 --l1-contracts-json-path ../../.devnet/sdk-addresses.json

[tynes]

https://github.com/ethereum-optimism/optimism/pull/3827 fixes the linting error in this PR

[tynes]

Unclear to me why this is failing, need to investigate more:

timeout 5m npx hardhat deposit-erc20 --network devnetL1 --l1-contracts-json-path ../../.devnet/sdk-addresses.json

This does not fail for me locally, will try again with a freshly cloned repo such that its closer to what runs in CI

[mslipper]

The devnet issues are unrelated - they work in CI when I activated SSH. I'm going to merge this in so that this doesn't cause conflicts. I'll look into the devnet issues.