feat(cli): add safe-propose command for Safe multisig CCIP proposals#243
Open
SyedAsadKazmi wants to merge 3 commits into
Open
feat(cli): add safe-propose command for Safe multisig CCIP proposals#243SyedAsadKazmi wants to merge 3 commits into
SyedAsadKazmi wants to merge 3 commits into
Conversation
SyedAsadKazmi
requested review from
a team,
PabloMansanet and
aelmanaa
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report |
andrevmatos
requested changes
May 3, 2026
Collaborator
There was a problem hiding this comment.
I think this deserves to be an option for tx-sending commands, instead of its own command;
so probably a --safe <safeAddress> option side-by-side with wherever --wallet is defined/called.
In practice, you still need the current/existing --wallet logic, to load the account/signer with which you'll effectively sign the proposal and send the on-chain tx to Safe's inbox.
But then, the tx-sending logic can/should be "wrapped" by safe logic, caching its nonce locally and, instead of sending the tx to e.g. Router, it'd wrap the unsigned tx and send it to Safe instead as a proposal.
Everything should work mostly as is, with the only considerations being, when you read back the result, we may need to adjust the expectation that the tx contains a CCIP request (e.g. in the end of the EVMChain.sendMessage method), and also that simulation/estimateGas will always succeed or provide accurate gasLimit (due to ccipSend being simulated strictly after approvals are mined). But I think it should be feasible to work around or special those cases when --safe is provided
| * Extracts the first available RPC URL from CLI args, RPC_* env vars, or rpcsFile. | ||
| * Needed to initialise the Safe Protocol Kit, which requires a single concrete URL. | ||
| */ | ||
| function extractFirstRpcUrl(argv: { rpcs?: string[]; rpcsFile?: string }): string { |
Collaborator
There was a problem hiding this comment.
Please, don't. Prefer to use source.provider directly if needed. Does safe libs have some form to integrate directly with ethers' providers? Would be even better than assuming a RPC url.
| // 12. Initialise the Safe Protocol Kit | ||
| const protocolKit = await SafeSDK.init({ | ||
| provider: rpcUrl, | ||
| signer: proposerPrivateKey, |
Collaborator
There was a problem hiding this comment.
Shouldn't either assume a private key; wallet could be a Ledger wallet.
Instead, let's make sure this works and simply wraps the discovered signer.
| // CJS interop issue: @safe-global packages have no "type":"module", causing TS to | ||
| // resolve the module namespace as the default export type, making it circular. | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment | ||
| const safeModule = (await import('@safe-global/protocol-kit')) as any |
Collaborator
There was a problem hiding this comment.
Please, let's also avoid those mixed imports; there's usually some way to import them in nodejs, even if it requires importing or accessing explicit defaults properties
| * @example | ||
| * `assert.eq(parseExtraArgs(['ccvs=["0xvalue1", "0xvalue2"]', 'finality=safe', 'flag=true']), { ccvs: ["0xvalue1", "0xvalue2"], finality: 'safe', flag: true })` | ||
| */ | ||
| function parseExtraArgs(extra: readonly string[] | undefined): Record<string, unknown> { |
Collaborator
There was a problem hiding this comment.
Shouldn't need this move if safe is built as a wrapper for wallet instead
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
ccip-cli safe-propose— proposes a CCIP send to the Safe Transaction Service queue without on-chain execution, so other Safe owners can review and sign in the Safe UI.ccipSendsignTransaction()--format jsonsupportedparseExtraArgsmoved toutils.ts;RPCS_REexported fromproviders/index.tssafe-propose.mdxreference page, sidebar entry, and commands table rowDependencies added (
ccip-cli)@safe-global/protocol-kit^7.1.0@safe-global/api-kit^4.1.0