Skip to content

Add RedeploySettlers + revive_chain.sh for reviving abandoned chains - #573

Open
jparklev wants to merge 11 commits into
masterfrom
jparklev/redeploy-settlers-revive-chain
Open

Add RedeploySettlers + revive_chain.sh for reviving abandoned chains#573
jparklev wants to merge 11 commits into
masterfrom
jparklev/redeploy-settlers-revive-chain

Conversation

@jparklev

@jparklev jparklev commented May 28, 2026

Copy link
Copy Markdown
Contributor

A refactoring #564 to be chain-agnostic + support eraVM

@jparklev
jparklev requested a review from duncancmt as a code owner May 28, 2026 22:56
@immunefi-magnus

Copy link
Copy Markdown

🛡️ Immunefi PR Reviews

We noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below:

🔗 Send this PR in for review

Once submitted, we'll take care of assigning a reviewer and follow up here.

@duncancmt

Copy link
Copy Markdown
Collaborator

@jparklev could you merge #590 into this branch and add the step to switch the upgrade Safe{Wallet} from v1.3.0 to v1.4.1 if it isn't already on v1.4.1?

@jparklev

Copy link
Copy Markdown
Contributor Author

@jparklev could you merge #590 into this branch and add the step to switch the upgrade Safe{Wallet} from v1.3.0 to v1.4.1 if it isn't already on v1.4.1?

Sure, will do 👌

@jparklev
jparklev requested a review from dekz as a code owner July 6, 2026 17:16

@duncancmt duncancmt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should deploy_safe_infra.sh also check and deploy v1.4.1?

Does this work like it should on weirdo chains like Tempo?

Comment thread sh/revive_chain.sh
Comment on lines +177 to +179
if [[ $safe_migration == null ]] ; then
safe_migration=0x0000000000000000000000000000000000000000
fi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem like a safe default

Comment thread sh/revive_chain.sh
required="$(bc <<<"$gas_price * ${sender_gas[$sender]}")"
actual="$(cast balance --rpc-url "$rpc_url" "$sender")"
if (( $(bc <<<"$actual < $required") )) ; then
echo "Insufficient ETH at $sender ($actual wei, need >= $required wei for ${sender_gas[$sender]} gas)" >&2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
echo "Insufficient ETH at $sender ($actual wei, need >= $required wei for ${sender_gas[$sender]} gas)" >&2
echo "Insufficient native asset at $sender ($actual wei, need >= $required wei for ${sender_gas[$sender]} gas)" >&2

ISafeMigration(safeMigration).SAFE_FALLBACK_HANDLER() == safeFallbackV141, "SafeMigration fallback mismatch"
);

_execTransaction(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this use _execDelegateCall?

safeBytecodes: SafeBytecodes("", "", "", "", "", "", "", "")
});
safeCompatConfig.safeBytecodes.load(vm);
if (safeCompatConfig.isEraVm) safeCompatConfig.safeBytecodes.loadV141(vm);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's a bit confusing. Why do we only load v1.4.1 when it's EraVm? I think this warrants a comment

Comment on lines +200 to +203
require(predictedTakerSubmittedSettler.code.length == 0, "predicted taker settler slot occupied");
require(predictedMetaTxSettler.code.length == 0, "predicted metatx settler slot occupied");
require(predictedIntentSettler.code.length == 0, "predicted intent settler slot occupied");
require(predictedBridgeSettler.code.length == 0, "predicted bridge settler slot occupied");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is unnecessarily defensive

Comment on lines +245 to +248
bytes memory takerDeployTx = _wrapSingleMultisend(_encodeMultisend(deployerProxy, takerSubmittedDeployCall));
bytes memory metaTxDeployTx = _wrapSingleMultisend(_encodeMultisend(deployerProxy, metaTxDeployCall));
bytes memory intentDeployTx = _wrapSingleMultisend(_encodeMultisend(deployerProxy, intentDeployCall));
bytes memory bridgeDeployTx = _wrapSingleMultisend(_encodeMultisend(deployerProxy, bridgeDeployCall));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

_wrapSingleMultisend seems unnecessary

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.

2 participants