feat: add Tempo session credential lifecycle hooks - #710
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
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: a8c5eb0c32
ℹ️ 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".
| }) | ||
| assertCredentialSourceCanSpend({ chainId, channel, source: credentialSource }) | ||
| if (channel.finalized) throw new ChannelClosedError({ reason: 'channel is finalized' }) | ||
| const channelState = await Chain.getChannelState(client, channelId, escrow) |
There was a problem hiding this comment.
When the split hooks are installed, Mppx invokes validate before broadcast on every credential-bearing request, so this unconditional getChannelState makes every normal voucher request depend on a fresh RPC eth_call even when channelStateTtl was configured to rely on the cached store state. The existing broadcast path still has the TTL guard, so high-frequency session traffic now pays an extra chain read per voucher and can start failing during transient RPC outages despite a fresh cached channel state; validation should reuse the same TTL/cache inputs as broadcast or avoid the mandatory read on the hot path.
Useful? React with 👍 / 👎.
| /** Parsed client-signed Tempo transaction. */ | ||
| transaction: ReturnType<(typeof Transaction)['deserialize']> | ||
| }): Promise<void> { | ||
| if (parameters.feePayer) return |
There was a problem hiding this comment.
When a session is configured with a local fee payer or hosted fee payer, this early return makes the validation hook accept sponsored open/topUp credentials without checking the sponsorship constraints that broadcast later enforces (for example an explicitly disallowed feeToken, expired validBefore, or gas values outside feePayerPolicy). A caller using mppx.validateCredential() can therefore receive a successful validation for a credential that the same method rejects during broadcast, so sponsored validation should run the non-mutating fee-payer policy/fee-token checks instead of unconditionally skipping them.
Useful? React with 👍 / 👎.
| await Chain.simulateCredentialTransaction({ | ||
| client, | ||
| feePayer: parameters.feePayer, | ||
| transaction: transaction.transaction, | ||
| }) |
There was a problem hiding this comment.
For top-up credentials this simulation is the final validation step, so mppx.validateCredential() can return success even when the channel is already pending close on-chain; the broadcast path rejects that same case after readback with validateChannelState(state). Callers that use validation as a preflight for top-up management will get a false positive for channels with closeRequestedAt != 0, so validation should read/validate the channel state (or at least reject known closed/pending-close store state) before accepting the credential.
Useful? React with 👍 / 👎.
1 participant
Motivation
Expose non-mutating validation and terminal broadcast hooks for Tempo session credentials.
Summary
Key design considerations