feat(tempo): multisig - #5029
Conversation
🦋 Changeset detectedLatest commit: 7292473 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a485f97064
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = await options.next({ | ||
| method: | ||
| options.method === 'eth_approveMultisigTransaction' | ||
| ? 'eth_sendRawTransaction' | ||
| : 'eth_sendRawTransactionSync', |
There was a problem hiding this comment.
If the downstream node accepts the transaction but the response is lost through a timeout or disconnect, this await throws before the operation is marked successful. The store therefore continues reporting pending even though the transaction may have executed, and later approvals rebroadcast the same envelope and can fail with an already-known or consumed-nonce error. Derive and persist the transaction identity before broadcasting, or reconcile ambiguous failures before leaving the operation pending.
Useful? React with 👍 / 👎.
| request: Multisig.handleRequest( | ||
| (request) => value.request(request as never), | ||
| parameters, | ||
| ) as typeof value.request, |
There was a problem hiding this comment.
For every client configured with multisig, the replacement request function forwards only the request object and discards the optional second argument accepted by EIP1193RequestFn. Consequently abort signals and per-request retryCount or dedupe overrides are ignored even for unrelated RPC methods; for example, cancelling client.request(..., { signal }) no longer reaches the underlying transport. Accept and forward the request options alongside the request.
Useful? React with 👍 / 👎.
| export function deserialize(value: string): Operation { | ||
| if (value.length > maxStoredValueLength) throw new InvalidStoreValueError() | ||
| const operation = Json.parse(value) as Operation | ||
| const config = MultisigConfig.from(operation.config) |
There was a problem hiding this comment.
When a custom or persistent store contains an operation with a missing or non-1 schemaVersion, this cast accepts it and proceeds to interpret it as the current layout. That defeats the version discriminator and can return or update an incompatible operation instead of raising InvalidStoreValueError; validate schemaVersion before constructing the configuration or transaction.
Useful? React with 👍 / 👎.
| export * as Formatters from './Formatters.js' | ||
| export * as Hardfork from './Hardfork.js' | ||
| export * as KeyAuthorizationManager from './KeyAuthorizationManager.js' | ||
| export * as Multisig from './Multisig.js' |
There was a problem hiding this comment.
This adds the public Multisig namespace without the required entrypoint TSDoc block, leaving the namespace undocumented in generated API and editor surfaces. Add a concise TSDoc description immediately above the export as required for new public modules.
AGENTS.md reference: AGENTS.md:L89-L93
Useful? React with 👍 / 👎.
| if (value === null) return null | ||
| return transactionOperation(value) |
There was a problem hiding this comment.
When the configured store or remote endpoint returns a key-authorization operation, this routes it through transactionOperation, which deliberately throws whenever keyAuthorization is present. As a result, getOperation only returns this advertised Operation.Operation variant when callers bypass the RPC path by passing store explicitly; deserialize the generic operation here instead.
Useful? React with 👍 / 👎.
| approveTransaction: ( | ||
| parameters: multisigActions.approveTransaction.Parameters<chain, account>, | ||
| ) => Promise<multisigActions.approveTransaction.ReturnValue> |
There was a problem hiding this comment.
These decorated methods instantiate Parameters with its broad default chainOverride and request types instead of declaring method-level generics, unlike the standalone functions and wallet decorators. Calls through client.multisig therefore lose literal request inference and compile-time chain-override checking, so incompatible builder output can type-check and fail only at runtime; make both approval methods generic over the request and chain override.
AGENTS.md reference: AGENTS.md:L68-L72
Useful? React with 👍 / 👎.
|
How do you handle transaction hashes not landed on chain yet? Ie what does eth get tx receipt return of anything? |
|
Also should nail the DX / UX of going through the 1 of 1 flow - since that will be very common by default as well as going from 1 of 1 to 1 of 2 when upgrading an agent account to a passkey backed or otherwise account (maybe using prf) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f547f1f990
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * @param parameters - Transaction request, multisig identity, and owner account. | ||
| * @returns The pending or successful multisig operation and prepared request. | ||
| */ | ||
| export async function approveTransaction< |
There was a problem hiding this comment.
The new approveTransaction, approveTransactionSync, and getOperation public actions have no corresponding pages under site/pages/tempo/actions, entries in the actions index, or sidebar wiring in site/vocs.config.ts. Users browsing the documented Tempo action surface therefore cannot discover or reference these APIs; add the action pages and wire them into the index and sidebar.
AGENTS.md reference: AGENTS.md:L103-L104
Useful? React with 👍 / 👎.
| keyAuthorization?: KeyAuthorization.Signed<quantity, index> | undefined | ||
| multisig?: Address | MultisigConfig.Config<index> | undefined | ||
| /** Bootstrap multisig config hint for node-side gas modeling (TIP-1061). Attached automatically when `multisig` is present; the node ignores it for registered senders. */ | ||
| multisigInit?: |
There was a problem hiding this comment.
This change removes the only descriptions of multisigInit, multisigOwnerStates, multisigSignatureCount, and multisigVersion while leaving them as public properties of TransactionRequestTempo. Generated API/editor documentation will now expose these non-obvious protocol fields without explaining their semantics, so retain TSDoc on each property.
AGENTS.md reference: AGENTS.md:L117-L118
Useful? React with 👍 / 👎.
| signatures: nested.flatMap((signature) => | ||
| signature.signatures.map((approval) => | ||
| SignatureEnvelope.serialize(approval), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
When a nested multisig owner's configuration changes while a parent operation is pending, this flattens every previously stored child signature and verifies it using the child's newly fetched version. The old signatures were made for the prior version and fail verification, and because they remain in existing.approvals, submitting a new valid child approval fails on every retry, permanently wedging that parent operation unless the store is manually altered. Track the child version or replace/reset stale nested approvals when the nested configuration changes.
Useful? React with 👍 / 👎.
| request: Multisig.handleRequest( | ||
| (request) => value.request(request as never), | ||
| parameters, | ||
| ) as typeof value.request, |
There was a problem hiding this comment.
When the supplied transport uses http(..., { raw: true }), value.request returns { result, error } envelopes, but the multisig handler treats those envelopes as decoded RPC values and returns plain strings or objects for intercepted methods. Consequently approval submissions fail while decoding getConfig, and direct eth_getMultisigOperation requests violate the raw transport's declared response shape. The wrapper must unwrap downstream raw responses for its internal actions and rewrap intercepted results, or reject raw transports explicitly.
Useful? React with 👍 / 👎.
| serializedTransactions.push(serialized) | ||
| if (serializedTransactions.length === 2) release?.() | ||
| await thresholdCrossed |
There was a problem hiding this comment.
This test releases thresholdCrossed only after recording two eth_sendRawTransactionSync calls, so an implementation that correctly elects a single broadcaster will hang forever rather than demonstrate correct observable behavior. It also asserts the internal request count and payload instead of the resulting operation and chain state; restructure the synchronization around observable outcomes without requiring duplicate downstream calls.
AGENTS.md reference: AGENTS.md:L187-L192
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f04b9d8d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async (existing) => { | ||
| if (existing?.keyAuthorization) throw new Store.InvalidStoreValueError() | ||
| if (existing?.status === 'success') return existing | ||
| if (existing && existing.weight >= existing.threshold) return existing |
There was a problem hiding this comment.
When a sponsored transaction reaches quorum but the downstream broadcast rejects its fee-payer signature, resubmitting an approval with a corrected feePayerSignature cannot recover the operation. The fee-payer signature is outside the sender approval digest, so the corrected envelope resolves to the same operation ID, but this early return retains the previously stored transaction and rebroadcasts its stale sponsorship data on every retry. Allow a quorum-complete pending operation to replace non-sender envelope data after validating that its approval payload is unchanged.
Useful? React with 👍 / 👎.
| if (typeof account === 'object' && account.source === 'multisig') | ||
| return account as TempoAccount.MultisigAccount | ||
| if (parameters.multisig) | ||
| return TempoAccount.fromMultisig(parameters.multisig) |
There was a problem hiding this comment.
When a local multisig account is approving a different parent multisig supplied as an address or raw config, this branch selects the signing child as multisigAccount before considering the explicit parameters.multisig. The subsequent overrides therefore prepare and submit the transaction from the child account rather than the requested parent, potentially executing the calls against the wrong account; explicit multisig identities should take precedence regardless of whether they are addresses, configs, or MultisigAccount objects.
Useful? React with 👍 / 👎.
| const signature = await signTransaction(client, { | ||
| ...request, | ||
| account, | ||
| } as never) |
There was a problem hiding this comment.
When account is an address, which the public parameter type explicitly permits, this calls the generic signTransaction JSON-RPC path. That path invokes eth_signTransaction and returns a complete transaction envelope from the address, not the primitive signature over the multisig approval digest expected by serializeApproval; the envelope is then treated as an owner signature and approval construction fails. Either restrict approval owners to local accounts or implement a JSON-RPC signing flow that signs the derived multisig digest.
Useful? React with 👍 / 👎.
|
|
||
| `viem/tempo`: Added persistent multisig operation coordination and explicit owner approval actions. | ||
|
|
||
| ```ts |
There was a problem hiding this comment.
The changeset continues after its past-tense summary with a fenced usage example, even though repository changeset entries must consist of a single sentence. Remove the example and retain only the summary sentence.
AGENTS.md reference: AGENTS.md:L224-L227
Useful? React with 👍 / 👎.
jxom
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a250139c3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const address = (() => { | ||
| if (account) return account.address | ||
| if (typeof accountValue === 'string') return accountValue as Address | ||
| return undefined |
There was a problem hiding this comment.
When account is supplied as a JsonRpcAccount object with currentConfig, this resolver ignores its address, and the destructured override is never restored in the call below. The action consequently uses client.account instead, so clients with a different default account either submit the config update from the wrong sender or fail during multisig preparation. Preserve non-multisig account objects as the explicit override rather than dropping them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 339187fccf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (request.owner) { | ||
| const owner = parseAccount(request.owner) | ||
| if (owner.type !== 'local') | ||
| throw new Error( | ||
| 'A local owner account is required to approve a multisig transaction.', | ||
| ) | ||
| if (owner.source !== 'root' && owner.source !== 'multisig') | ||
| throw new Error( | ||
| 'A Tempo owner account is required to approve a multisig transaction.', | ||
| ) | ||
| const { owner: _, ...ownerRequest } = request | ||
| return await owner.signTransaction(ownerRequest as never, options) |
There was a problem hiding this comment.
When the public signTransaction action is called with a multisig account and the new owner option, it does not run the chain's preparation hook, so multisigSimulation is normally absent. This branch nevertheless delegates the request to the owner, which produces an ordinary Tempo transaction whose recovered sender is the owner rather than the requested multisig; broadcasting it can execute the calls with the wrong account's authority and balance. Require a prepared multisig simulation context here instead of falling back to ordinary owner signing.
Useful? React with 👍 / 👎.
| return SignatureEnvelope.serialize( | ||
| await signMultisig(account, { | ||
| payload: parentDigest, | ||
| states: request.multisigOwnerStates, | ||
| version: state?.version, | ||
| }), |
There was a problem hiding this comment.
When a nested multisig owner is itself distributed and the local participant holds less than the child's threshold, this call reaches signMultisig, which throws Local multisig owners do not meet the threshold instead of returning the partial nested envelope. Consequently the coordinator never receives either child's partial approval and cannot merge them into the nested branch, despite the documented coordinated flow in site/pages/tempo/guides/multisig/nested-accounts.mdx:87-89; support emitting partial nested approvals when signing a parent operation.
Useful? React with 👍 / 👎.
| }) | ||
| ``` | ||
|
|
||
| ## How it works |
There was a problem hiding this comment.
Rename this heading to ## How It Works; the new page currently uses sentence case even though all documentation headings must use Title Case.
AGENTS.md reference: AGENTS.md:L248-L258
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7cd17bbe9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const configCommitment = await validateConfig({ | ||
| account: signature.account, | ||
| blockNumber, | ||
| client: options.client, | ||
| config, | ||
| }) |
There was a problem hiding this comment.
When the successful operation updates its own multisig config, or the config is rotated afterward, retrying the operation after a lost client response revalidates the old signed config against the new onchain commitment here and throws before OperationStore.update can return its existing success record. This makes an already-completed operation non-idempotent, including retries made with its documented operation hash; compute the hash and return a matching stored success before applying current-config validation.
Useful? React with 👍 / 👎.
| const submittedHash = await OperationStore.readSubmission( | ||
| options.store, | ||
| claim, | ||
| submissionId, | ||
| ) |
There was a problem hiding this comment.
When the downstream relay transforms the envelope before broadcasting, such as by adding sponsorship, the normal path explicitly accepts a returned hash different from the locally derived hash at lines 460-462. If that broadcast succeeds but its response is lost, this recovery path queries only the hash of the persisted pre-transform envelope, cannot find the mined transaction, releases the lease, and may repeatedly rebroadcast or leave the operation pending. Recovery needs a relay lookup or persisted identifier that resolves the actual transformed transaction rather than assuming the pre-broadcast hash.
Useful? React with 👍 / 👎.
| const { owner: _, ...ownerRequest } = request | ||
| return await owner.signTransaction(ownerRequest as never, options) |
There was a problem hiding this comment.
When a transaction request supplies both owner and existing signatures, this delegation forwards those signatures into the owner's serializer. The owner therefore hashes an envelope that already contains the earlier multisig approvals, while transactionEnvelope later appends the new approval to the original unsigned operation; the coordinator verifies that approval against the unsigned payload and rejects it. Remove signatures from the request used for owner signing, while retaining them for final envelope assembly.
Useful? React with 👍 / 👎.
| const resolvedAccount = (() => { | ||
| if (account) return { ...account, config: currentConfig } | ||
| if (typeof accountValue === 'object') return accountValue | ||
| if (address) return fromMultisig({ address, ...currentConfig }) |
There was a problem hiding this comment.
When updateConfig receives an address account and resolves its cached initial config, that config has version 0, so this call to fromMultisig({ address, ...currentConfig }) throws A current multisig config must have a version. before the supplied owner can coordinate the account's first rotation. This contradicts the action's cached-config inference path specifically for initial accounts; construct version-zero configs with address: 'infer' and verify the derived address matches, as the coordinated key-authorization path already does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a22eeba61a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| quantity = bigint, | ||
| index = number, | ||
| status = 'success' | 'reverted', | ||
| status = 'success' | 'reverted' | 'pending', |
There was a problem hiding this comment.
When a distributed multisig has not reached quorum, pendingResult returns null for blockHash, blockNumber, cumulativeGasUsed, effectiveGasPrice, gasUsed, logsBloom, and transactionIndex, but adding 'pending' only to this status parameter leaves those fields non-nullable through viem_TransactionReceipt. TypeScript callers can therefore perform bigint or hash operations on values that are actually null; model pending receipts as a discriminated variant with nullable inclusion fields and add a type test for the narrowing.
AGENTS.md reference: AGENTS.md:L75-L76
Useful? React with 👍 / 👎.
| These recipes assume you have [set up a Tempo client](/tempo). | ||
|
|
||
| ### Combine Owner Key Types | ||
| ### Approve from Different Key Types |
There was a problem hiding this comment.
This guide now moves directly from ## Recipes to the first recipe, removing the required prerequisite line that links readers to Tempo client setup. Restore that line before this heading so the recipe's imported client has the required setup context.
AGENTS.md reference: AGENTS.md:L352-L354
Useful? React with 👍 / 👎.
| if (existing?.status === 'submitting' && existing.expiresAt! > now) | ||
| return existing | ||
| const existingApprovals = existing |
There was a problem hiding this comment.
When a coordinator exits after writeSubmission but before broadcasting or cleanup, the lease eventually expires and this path replaces the submitting operation with a new pending operation without removing the artifact keyed by the old submissionId. That identifier is then lost, so every such recovery can permanently leak another final envelope of up to 1 MiB into the shared store; retain the expired ID long enough to remove its submission key before starting a new attempt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 202a332c04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (value.address !== 'infer' && config!.version === 0n) | ||
| throw new Error('A current multisig config must have a version.') |
There was a problem hiding this comment.
Existing calls such as Account.fromMultisig({ owners: [...] }) now produce a version-zero config with value.address === undefined, enter this branch, and throw A current multisig config must have a version. at runtime; typed consumers also stop compiling because the public Config shape was removed. Since the commit publishes a patch changeset, continue treating an omitted address as inferred, or release and document this as a breaking migration.
AGENTS.md reference: AGENTS.md:L224-L227
Useful? React with 👍 / 👎.
| /** @experimental */ | ||
| export * as Selectors from './Selectors.js' | ||
| export * as Storage from './Storage.js' | ||
| export * as Store from './Store.js' |
There was a problem hiding this comment.
Replacing this export removes the previously public Storage namespace and deletes its module, so existing import { Storage } from 'viem/tempo' consumers fail to compile after a patch upgrade even though Store exposes essentially the renamed API. Keep a deprecated Storage alias, including defaultStorage, or publish this as a documented breaking migration.
AGENTS.md reference: AGENTS.md:L224-L227
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2f8c9ec4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| request.nonceKey = Hex.toBigInt(Hex.random(31)) + 1n | ||
| request.nonce = 0 |
There was a problem hiding this comment.
When a coordinated multisig caller supplies nonce but leaves nonceKey at its normal default, this branch silently replaces both the default nonce lane and the caller's nonce with a random lane at nonce zero. The resulting operation can execute outside the caller's intended sequence; only allocate the random lane when neither nonce field was explicitly provided.
Useful? React with 👍 / 👎.
| createTransport({ | ||
| key: 'recording', | ||
| name: 'Recording', | ||
| request: async () => 'tempo' as never, |
There was a problem hiding this comment.
This newly added request-options test uses a hand-built transport stub whose request function returns a fixed value, rather than exercising behavior against the configured test chain or a real ephemeral server. Rework the test around a real endpoint so it complies with the repository's explicit prohibition on mocks, stubs, and fake clients.
AGENTS.md reference: AGENTS.md:L146-L149
Useful? React with 👍 / 👎.
| multisigVersion?: bigint | undefined | ||
| multisigSimulation?: MultisigSimulation.Spec | undefined | ||
| nonceKey?: 'expiring' | quantity | undefined | ||
| owner?: Account | MultisigAccount | Address | undefined |
There was a problem hiding this comment.
The public transaction type permits an address or any Tempo Account as owner, but request preparation unconditionally rejects string owners and account sources other than root or multisig. Calls such as sendTransaction({ account: multisig, owner: owner.address, ... }) therefore compile and then fail before signing; narrow both request and serializable owner fields to RootAccount | MultisigAccount.
AGENTS.md reference: AGENTS.md:L75-L76
Useful? React with 👍 / 👎.
| createdAt: existing?.createdAt ?? now, | ||
| hash: operationHash, | ||
| signatureCount: approvals.signatureCount, | ||
| status: 'pending', |
There was a problem hiding this comment.
When this handler is exposed to untrusted RPC callers, an attacker can repeatedly submit one valid approval for unique version-zero 2-of-2 accounts and transactions; each request passes initial-config validation and is permanently persisted with pending status here. Operation keys have no TTL, quota, or removal path (only temporary submission artifacts are removed), so the shared coordinator store can be exhausted without any transaction reaching quorum; enforce retention limits or expire incomplete operations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f1fda6fbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ((receipt as TransactionReceipt).status === 'pending') | ||
| return { receipt } as never |
There was a problem hiding this comment.
When a coordinated multisig has not reached quorum, this branch returns only { receipt }, but authorizeSync.ReturnValue still requires every KeyAuthorized event field. TypeScript therefore lets callers read fields such as account that are actually undefined. The same unchecked pending branch was added across the other *Sync action wrappers, so their public return types should include a pending variant discriminated by receipt.status, with type tests covering both outcomes.
AGENTS.md reference: AGENTS.md:L75-L76
Useful? React with 👍 / 👎.
| async sign({ hash }) { | ||
| return SignatureEnvelope.serialize( | ||
| await signMultisig(account, { payload: hash }), | ||
| ) |
There was a problem hiding this comment.
This implementation makes MultisigAccount.sign functional when the account has a sufficient local owner quorum, but the updated site/pages/tempo/accounts/account.fromMultisig.mdx:147-149 still explicitly says that sign is unsupported. Users relying on that public account reference will miss the signing capability used by nested multisig and coordinated authorization flows, so update the page and the source TSDoc to describe its quorum/config requirements.
AGENTS.md reference: AGENTS.md:L116-L116
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f1fda6fbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ((receipt as TransactionReceipt).status === 'pending') | ||
| return { receipt } as never |
There was a problem hiding this comment.
When a distributed multisig has not reached quorum, this branch returns only { receipt }, but authorizeSync.ReturnValue still requires every KeyAuthorized event field. Typed callers can therefore use fields such as the authorized account as defined values even though they are absent at runtime. Return a discriminated pending variant instead of hiding the different shape behind as never; the same pattern should be corrected in the other modified sync actions.
AGENTS.md reference: AGENTS.md:L74-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 729247317a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const key = signature.account.toLowerCase() | ||
| configs.set(key, { account: signature.account, config }) |
There was a problem hiding this comment.
When separate branches of a nested approval tree reuse the same multisig account but carry different config versions, this address-only map lets the later branch overwrite the earlier config before onchain validation. MultisigOperation.selectApprovals verifies each branch against its supplied config, so a stale branch can still contribute weight while only the other branch's current config is checked, producing an invalid transaction or a falsely successful key-authorization operation. Key and validate entries by both account and config commitment, or reject conflicting configs for the same account.
Useful? React with 👍 / 👎.
| [`Store.memory`](/tempo/utilities/Store.memory) for process-local data or | ||
| [`Store.from`](/tempo/utilities/Store.from) to wrap another store. | ||
|
|
||
| ## Contract |
There was a problem hiding this comment.
This new Store module landing page jumps directly into ## Contract and later ## See Also, omitting the required ## Overview, minimal usage example, and Cards grid linking its from, memory, session, and defaultStore sub-pages. Rework the page into the prescribed concept-first landing-page structure.
AGENTS.md reference: AGENTS.md:L267-L275
Useful? React with 👍 / 👎.
| const localOwner = local?.owners.find((account) => | ||
| isAddressEqual(account.address, owner.owner), | ||
| ) |
There was a problem hiding this comment.
When a coordinated request uses an address-only parent account and passes a MultisigAccount through owner, the nested account is not present in local.owners, so this lookup models it as one primitive WebAuthn approval instead of a nested quorum. Both initial submissions and operation-hash continuations can therefore receive a gas limit based on a much smaller envelope than the one ultimately serialized, causing an otherwise valid nested approval to run out of gas; include the explicit owner when resolving simulation accounts.
Useful? React with 👍 / 👎.
3 participants
Adds persistent multisig operation storage and explicit approval actions to Tempo clients. This keeps partial quorum coordination out of standard transaction sending while preserving direct sends for complete multisig envelopes.
Supersedes #5026.