Add PausableSapient support to Trails intents - #367
Conversation
The wrapper hardcodes checkpoint=0, which no real caller can use (intent wallets need a salt-derived checkpoint), and has no production callers anywhere in the workspace — only its own test and two trails-watchtower test files exercise it. Callers build the timed-refund sapient leaf directly via TimedRefundSapientImageHash and pass it to CreateIntentConfiguration, which remains unchanged.
…Configuration CreateIntentTree and CreateIntentConfiguration take a new payloadGateLeafNode parameter: when set, the wallet is satisfied by [calls && payloadGateLeafNode] signing together (a 2-of-2 subtree that caps the calls' any-address-subdigest leaves' own, otherwise uncapped, weight) OR by sapientSignerLeafNode, untouched. This lets a caller gate payload execution behind a revocable signer (e.g. a pausable contract) while leaving other leaves (e.g. a timed-refund signer) unaffected. Passing nil preserves the exact legacy tree shape, so already-derived counterfactual addresses do not change.
|
Confirming we only want the calls to be pausable (current implementation). I think making the the deposit pausable under the same lock makes sense. If execution is blocked, there is no reason we should have deposits enabled too. Locking the timed refund sapient with the same lock, I'm less sure about. If the argument for using the lock is "Polygon contracts may be exploitable" then it makes sense. But blocking automated refund capability is iffy. Note that in either case the owner will still have 1/1 signing power and can do whatever they want with the intent. Update: Confirmed we gate all sapients too. Only the owner remains ungated. |
CreateIntentTree/CreateIntentConfiguration now wrap sapientSignerLeafNode in its own independent 2-of-2 subtree with payloadGateLeafNode, the same way the calls leaves already are, when both are provided. A deposit or timed-refund sapient leaf is a payload-execution path just like the calls leaves, so withholding the gate leaf's signature blocks it too. mainSigner is never gated, so the owner can always act (e.g. recover funds) regardless of the gate's state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 4b4377dc4c
ℹ️ 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".
A sapient-only config (empty calls) left wrapPayloadGate's inner threshold-1 node wrapping zero protected leaves, producing a NestedLeaf with a nil Tree that panics on ImageHash or any other tree traversal. Omit the calls gate entirely when there are no calls to gate, leaving the separately gated sapient leaf intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
payloadGateLeafNode is an opaque, caller-supplied v3.WalletConfigTree. Inserted raw, a misweighted or malicious leaf (weight >= 2) could meet wrapPayloadGate's threshold-2 requirement on its own, with the protected leaves contributing nothing — silently collapsing "calls && gate" down to "gate alone". Wrap it in its own weight-1 nested leaf so its contribution is capped regardless of its declared weight. Also rewrites the sapient-leaf-gated signature test to check each leaf's wiring independently instead of combining two signatures in one BuildIntentConfigurationSignature call: once the gate leaf alone already meets the wallet's overall threshold (via the calls gate's auto-satisfying any-address-subdigest leaf), that combination races BuildRegularSignature's early-cancellation against collecting the other signer's signature, an existing behavior unrelated to this fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Share one OR-nest gate (threshold = gateWeight+1) so either group needs the payload gate's co-signature, without separate per-leaf gates or weight-1 rejection.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1286b4b4
ℹ️ 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".
| gateableTree := &v3.WalletConfigTreeNestedLeaf{ | ||
| Weight: 1, | ||
| Threshold: 1, | ||
| Tree: v3.WalletConfigTreeNodes(gateableLeaves...), |
There was a problem hiding this comment.
When building a signature for the sapient alternative against a payload that does not match any configured call digest, this shared inner tree causes WalletConfigTreeAnyAddressSubdigestLeaf.signersWeight to report maximum weight without checking the payload. If the payload-gate signer responds first, BuildRegularSignature therefore considers the outer gate satisfied and cancels the other signing goroutines, so the supplied sapient signature can be omitted nondeterministically; recovery then gives the call leaf zero weight and rejects the otherwise valid gate-plus-sapient authorization. Ensure signature construction embeds the required supplied sapient signature rather than letting the payload-independent call leaf trigger early cancellation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is incorrect. Any gated leaf is able to satisfy the nested tree leaf. When building the signature, unused nodes are ignored from validation so the zero weight case is impossible.
There was a problem hiding this comment.
I reproduced this. With both signatures supplied, 221/1000 builds were byte-for-byte gate-only. We should not cancel explicitly provided signatures here.
There was a problem hiding this comment.
Thanks for verifying! Fixing. This gap is upstream in existing code. I'm putting a fix here for Trails only and making a follow up PR to address upstream.
| // Add the sapient signer leaf to the gateable leaves. | ||
| if sapientSignerLeafNode != nil { | ||
| gateableLeaves = append(gateableLeaves, sapientSignerLeafNode) |
There was a problem hiding this comment.
When payloadGateLeafNode and sapientSignerLeafNode use the same verifying contract address with different image hashes, the new combined configuration contains two distinct core.Signer values, but BuildIntentConfigurationSignature selects each supplied signature using only signer.Address. Both signing requests can consequently receive the first same-address signature, leaving one leaf encoded with a signature for the wrong image hash and making valid gate-plus-sapient authorization fail. Match the full signer identity, including sapient status and image hash, or reject this otherwise valid configuration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No this is a valid (though unlikely) case. A sapient may recover two different and valid image hashes when provided different signatures
There was a problem hiding this comment.
Yes, and that is why matching only Address is wrong here. We need to match the full core.Signer, including ImageHash.
There was a problem hiding this comment.
Thanks for verifying! Fixing. For additional context, this was a pre-existing bug only surfaced by this implementation. In practice, it would not be hit as the gate and sapient leaves will not use the same address (configured downstream in Trails).
| func CreateIntentTree( | ||
| mainSigner common.Address, | ||
| calls []*v3.CallsPayload, | ||
| payloadGateLeafNode v3.WalletConfigTree, |
There was a problem hiding this comment.
nil preserves the tree hash, but this still breaks every existing Go caller. Can we keep the old functions and add WithPayloadGate variants?
There was a problem hiding this comment.
We could. Breaking was an intention choice. This functions is specifically related to constructing a Trails intent. We expect Trails to use a value in this new field. Clients upgrading should be aware that their old implementation requires adjustment.
This is the same decision that was made when adding sapientSignerLeafNode.
There was a problem hiding this comment.
Thought about this more. I think my comment still stands but let's prevent future churn by adding optional vars here. Updated
leafWeight backs wrapPayloadGate's cap on payloadGateLeaf's contribution. Only terminal leaf types (address, sapient signer) have a weight that actually bounds what they contribute; a NestedLeaf's declared Weight doesn't bound its subtree, so accepting it let a caller understate the gate leaf's real contribution.
signingFunc matched signerSignatures by Address only, so two Signer values sharing an address but differing in IsSapient/ImageHash (e.g. a payload gate and a sapient signer leaf on the same contract) could both receive the first same-address signature, leaving one leaf signed for the wrong image hash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetIntentConfigurationSignature previously hardcoded nil for the payload gate leaf when building its intent configuration, so a caller with a pausable gate configured got a signature for the wrong (ungated) wallet config. Add the payloadGateLeafNode param, matching CreateIntentConfiguration's signature, and update all callers. Add subtests covering GetIntentConfigurationSignature with the gate alone and with the gate plus a sapient signer leaf.
…natures BuildIntentConfigurationSignature routed pre-collected signatures through BuildRegularSignature's signing orchestrator, which cancels outstanding signers once the config threshold looks met. Subdigest leaves report max weight regardless of payload, so a payload gate signature arriving first satisfied the threshold and nondeterministically dropped the sapient signature that recovery of a non-matching payload still needs. Add WalletConfig.BuildRegularSignatureFromSignatures, which builds the signature tree directly from the supplied signatures with no orchestration or cancellation, and use it in BuildIntentConfigurationSignature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add IntentConfigOption with WithPayloadGate and WithSapientSigner so CreateIntentTree, CreateIntentConfiguration and GetIntentConfigurationSignature take named options instead of adjacent positional WalletConfigTree params. Future optional leaves become additive options rather than signature breaks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6237ebec6f
ℹ️ 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".
An untyped nil is assignable to the variadic option type, so legacy calls passing nil for the removed positional leaf params compile and then panicked at invocation. Skip nil options instead; nil meant "no leaf" under the positional API and now means "no option", preserving identical behavior for those callers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e096336b8
ℹ️ 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".
A gated signer leaf sharing the gate's identity satisfies both sides of the outer threshold with one signature, letting the gate authorize alone. Replace leafWeight with signerLeaf, which also returns the leaf's signer identity, and reject configs where the gate signer appears among the gated leaves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Payload-matching leaves (subdigest and any-address-subdigest) carry no signer and match any weight requirement, so one used as the gate would authorize alone. Report them from signerLeaf with a signerless identity and maxUint256 weight, and cap the gate weight at maxUint64 so they are rejected before the threshold conversion can truncate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A typed-nil leaf pointer passes the interface nil check, so signerLeaf dereferenced it and CreateIntentConfiguration panicked instead of returning an invalid-gate error. Check the concrete pointer for nil in both signer cases before reading its fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cde50e6ac
ℹ️ 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".
The gate co-signs more than just the payload (e.g. sapient leaves), so drop the payload- prefix from the leaf, option, and helper names. Co-authored-by: Cursor <cursoragent@cursor.com>
Gated leaves may be nested trees, so check the gate identity against the full recursive signer set (WalletConfig.Signers) instead of only top-level terminal leaves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
Summary
CreateIntentTree/CreateIntentConfigurationvia theWithGateoption: when set, the calls and the sapient signer leaf (e.g. a timed-refund or gasless-deposit signer, added viaWithSapientSigner) share a single gate requiring the gate leaf's co-signature — either group alone, plus that co-signature, is sufficient (an inner threshold-1 OR-nest across both groups, wrapped in an outer threshold of the gate leaf's weight + 1).mainSigneris never gated, so the owner can always act (e.g. recover funds) regardless of the gate's state. OmittingWithGatekeeps the exact legacy tree shape. A sapient-only config (no calls) omits the calls side of the gate rather than producing a broken nil-tree nest.WithGate,WithSapientSigner) instead of positional parameters; nil options are ignored. The gate leaf is threaded throughGetIntentConfigurationSignature, and supplied signatures are embedded deterministically.WalletConfig.Signers()). A gated occurrence of the gate's identity — even buried in a nested leaf or node branch — would let a single signature satisfy both sides of the outer threshold, allowing the gate to authorize arbitrary payloads alone.CreateIntentConfigurationWithTimedRefundSapientwrapper (hardcodedcheckpoint=0, no production callers anywhere in the workspace — only test-only usage in this repo and in trails-watchtower).Intended consumer: gating v1.5 intent wallet execution (calls, gasless-deposit permit, and timed-refund) behind the
PausableSapientcontract, so pausing it blocks all of those while owner recovery keeps working.Test plan
go build ./...go test .(intent-config tests, full local suite against anvil) — all pass except a pre-existing, unrelatedTestWalletDeploy/v1failure (confirmed present onmastertoo)PausableSapientdeployment on Base🤖 Generated with Claude Code