fix(tempo): align native multisig with tip-1061 - #389
Conversation
|
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: 443e534f2c
ℹ️ 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".
| @@ -0,0 +1,11 @@ | |||
| --- | |||
| 'ox': patch | |||
There was a problem hiding this comment.
initialConfig rename as a breaking change
This patch removes the public genesisConfig input from both MultisigConfig.getSignPayload and SignatureEnvelope.from, so existing Ox 1.x consumers will fail to type-check—and untyped SignatureEnvelope.from({ genesisConfig, signatures }) calls will throw—after accepting a patch upgrade. Preserve genesisConfig as a compatibility alias or mark this as a major changeset and provide the required before/after diff migration fence.
AGENTS.md reference: AGENTS.md:L118-L121
Useful? React with 👍 / 👎.
| */ | ||
| export function getSignPayload(value: getSignPayload.Value): Hex.Hex { | ||
| const { payload } = value | ||
| const { payload, version = 0n } = value |
There was a problem hiding this comment.
When an account has undergone any config update, this default silently derives a version-0 digest unless the caller passes the current version. The shipped guide at site/src/pages/tempo/guides/transaction-envelopes/multisig-transactions.mdx:151-205 explicitly says its later-transaction flow works after the onchain config has been updated, but it calls both getSignPayload and sortMultisigApprovals without version; following that guide after the first update therefore produces approvals the node rejects. Update the guide to obtain and pass the current config version alongside this API change.
AGENTS.md reference: AGENTS.md:L64-L69
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 041c8a9dcf
ℹ️ 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".
|
|
||
| /** Maximum number of owners allowed in a native multisig config. */ | ||
| export const maxOwners = 255 | ||
| export const maxOwners = 50 |
There was a problem hiding this comment.
When users migrate a multisig with 51–255 owners, MultisigConfig.assert now rejects it because this constant is 50, but site/src/pages/migrating-from-v0.md:155 still explicitly documents MultisigConfig.maxOwners as 255. Update the migration guide alongside this protocol-limit change so it does not recommend configurations the library rejects.
AGENTS.md reference: AGENTS.md:L64-L67
Useful? React with 👍 / 👎.
| const accountBinding = | ||
| account !== undefined ? { account, isAdmin: isAdmin ?? false } : {} |
There was a problem hiding this comment.
When decoding an RPC authorization that contains account but omits isAdmin, this now preserves the binding and returns isAdmin: false; however, site/src/pages/tempo/guides/access-keys/admin.mdx:141-144 still says Ox drops orphan admin fields and instructs users to pair the account only with isAdmin: true. Update that guidance to explain that account-bound non-admin authorizations are now supported.
AGENTS.md reference: AGENTS.md:L64-L67
Useful? React with 👍 / 👎.
| export type Signature<numberType = number> = OneOf< | ||
| | SignatureEnvelope.Primitive<numberType> | ||
| | SignatureEnvelope.Multisig<numberType> | ||
| > |
There was a problem hiding this comment.
Now that key authorizations accept multisig signatures, the shipped multisig guide at site/src/pages/tempo/guides/transaction-envelopes/multisig-transactions.mdx:239-243 still tells users never to include keyAuthorization and claims native multisig accounts cannot use AccountKeychain. That directly contradicts the newly supported bootstrap and subsequent access-key flows, so update the guide to document the multisig authorization procedure instead of steering users away from it.
AGENTS.md reference: AGENTS.md:L64-L67
Useful? React with 👍 / 👎.
1 participant
Aligns native multisig signing and validation with the latest TIP-1061 implementation in tempoxyz/tempo#4069. Adds version binding, current limits, nested approval handling,
initialConfig, and config-rotation coverage.