Skip to content

U15 Mainnet Task #322

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jjtny1
Copy link
Contributor

@jjtny1 jjtny1 commented Apr 23, 2025

Mainnet smart contract update tasks for Isthmus: https://docs.optimism.io/notices/upgrade-15

@jjtny1 jjtny1 marked this pull request as draft April 23, 2025 17:19
@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Apr 23, 2025

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@@ -0,0 +1,18 @@
OP_COMMIT=c8b9f62736a7dad7e569719a84c406605f4472e6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,18 @@
OP_COMMIT=c8b9f62736a7dad7e569719a84c406605f4472e6
BASE_CONTRACTS_COMMIT=2d0adea2460e3419dbcd77ac48dd2efc9a9a375f
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest commit

BASE_CONTRACTS_COMMIT=2d0adea2460e3419dbcd77ac48dd2efc9a9a375f

ABSOLUTE_PRESTATE=0x03682932cec7ce0a3874b19675a6bbc923054a7b321efc7d3835187b172494b6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# COORDINATOR_SAFE_ADDR=0x646132a1667ca7ad00d36616afba1a28116c770a
# SAFE_A=0x5dfEB066334B67355A15dc9b67317fD2a2e1f77f
# SAFE_B=0x6af0674791925f767060dd52f7fb20984e8639d8
# LEDGER_ACCOUNT=1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove this

sign-cb:
$(GOPATH)/bin/eip712sign --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" -- \
forge script --rpc-url $(L1_RPC_URL) UpgradeDGF \
--sig "sign(address,address)" $(OWNER_SAFE) $(CB_SIGNER_SAFE_ADDR)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to change signature to sign(address,address) instead of sign(address).
Where can i get the second safe address from?

@jjtny1 jjtny1 marked this pull request as ready for review April 25, 2025 16:09
@jjtny1 jjtny1 requested review from anikaraghu and jackchuma April 25, 2025 16:09
@jjtny1 jjtny1 changed the title [DRAFT] - mainnet changes U15 Mainnet Task Apr 25, 2025
@jjtny1 jjtny1 requested a review from xenoliss April 25, 2025 16:12
Comment on lines +24 to +25
// TODO: Confirm expected version
string public constant EXPECTED_VERSION = "1.4.1";
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

@@ -0,0 +1,12 @@
OP_COMMIT=ef7a933ca7f3d27ac40406f87fea25e0c3ba2016
Copy link
Contributor

Choose a reason for hiding this comment

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

It does not match the commit we used on Sepolia, is it intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

This corresponds to op-contracts/v3.0.0-rc.2. Should be good https://github.com/ethereum-optimism/optimism/commits/op-contracts/v3.0.0-rc.2/

clockExtension: clockExtension,
maxClockDuration: maxClockDuration,
vm: bigStepper,
weth: currentFdg.weth(),
Copy link
Contributor

Choose a reason for hiding this comment

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

We could extract it as for the other params.

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's fine to leave it this way - the weth implementations are not the same between FaultDisputeGame and PermissionedDisputeGame

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, my bad.

address public pdgImpl;

constructor() {
OWNER_SAFE = vm.envAddress("OWNER_SAFE");
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also extract the SYSTEM_CONFIG so that we know all env vars are read in the constructor.

Comment on lines +110 to +111
// _postcheckHasAnchorState(CANNON);
// _postcheckHasAnchorState(PERMISSIONED_CANNON);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be uncommented.

using LibDuration for Duration;
using LibGameType for GameType;

// TODO: Confirm expected version
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably remove this comment

// TODO: Confirm expected version
string public constant EXPECTED_VERSION = "1.4.1";

SystemConfig internal _SYSTEM_CONFIG = SystemConfig(vm.envAddress("SYSTEM_CONFIG"));
Copy link
Contributor

Choose a reason for hiding this comment

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

We could assign this as a local variable in the setUp function

clockExtension: clockExtension,
maxClockDuration: maxClockDuration,
vm: bigStepper,
weth: currentFdg.weth(),
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's fine to leave it this way - the weth implementations are not the same between FaultDisputeGame and PermissionedDisputeGame

approve-op:
forge script --rpc-url $(L1_RPC_URL) UpgradeDGF \
--sig "approve(address[],bytes)" [$(OP_SIGNER_SAFE_ADDR)] $(SIGNATURES) \
--ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast
--ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast -vvvv

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