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

[Sepolia] Fix withdrawal network for fee vaults #122

Merged
merged 5 commits into from
Feb 2, 2024

Conversation

cody-wang-cb
Copy link
Contributor

@cody-wang-cb cody-wang-cb commented Feb 1, 2024

It turned out that the genesis fee vaults didn't have the correct value for withdrawal network, it should be L2 (1) instead of L1 (0). Thus updating the script to have the correct value.

Will need to rerun this script and verify the new contracts.

Performed a dry run and the script runs successfully

Script ran successfully.

== Logs ==
  Sequencer Fee Vault Impl address: 0x7ce53CAa10ae0090ce28276086eFaeADc15F71Df
  L1 Fee Vault Impl address: 0x9A33D2cac3cef6dC2F3e61c20417e78326d4Bdb7
  Base Fee Vault Impl address: 0x8C39078aEa04dEE6C8e2487BCfdB2b00d094b56C

## Setting up 1 EVM.

==========================

Chain 84532

Estimated gas price: 3.000000508 gwei

Estimated total gas used for script: 2121217

Estimated amount required: 0.006363652077578236 ETH

Change log to be approved: https://docs.google.com/document/d/1VTGKFdAq3BtoDPQMuH4E47g0t0u9H_fi1eo1RJMFOn8/edit?usp=sharing

@cody-wang-cb cody-wang-cb changed the title Fix withdrawl network for fee vaults [Sepolia] Fix withdrawl network for fee vaults Feb 1, 2024
@cody-wang-cb cody-wang-cb changed the title [Sepolia] Fix withdrawl network for fee vaults [Sepolia] Fix withdrawal network for fee vaults Feb 1, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense to remove this record or we should keep this?

Copy link
Contributor

Choose a reason for hiding this comment

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

i think it makes sense to keep it as a record

@@ -33,19 +33,19 @@ contract UpdateFeeVaultRecipient is Script {
SequencerFeeVault sfvNew = new SequencerFeeVault(
recipient,
sfvOld.MIN_WITHDRAWAL_AMOUNT(),
sfvOld.WITHDRAWAL_NETWORK()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the fact that this is wrong means there's some wrong configuration in the genesis file? Is it somewhere in the OP repo where we set this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mdehoog
mdehoog previously approved these changes Feb 1, 2024
@cb-heimdall cb-heimdall dismissed mdehoog’s stale review February 1, 2024 21:18

Approved review 1857415616 from mdehoog is now dismissed due to new commit. Re-request for approval.

@cody-wang-cb cody-wang-cb merged commit adb217c into main Feb 2, 2024
4 checks passed
@cody-wang-cb cody-wang-cb deleted the cody/fix_withdrawl_network branch February 2, 2024 18:43
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.

3 participants