diff --git a/.changeset/fix-session-signer-channel-reuse.md b/.changeset/fix-session-signer-channel-reuse.md new file mode 100644 index 00000000..d8914895 --- /dev/null +++ b/.changeset/fix-session-signer-channel-reuse.md @@ -0,0 +1,5 @@ +--- +'mppx': patch +--- + +Fixed legacy session auto-management reusing a cached channel after the account's `authorizedSigner` changed. The client now opens a new channel instead of emitting a voucher the escrow would reject. diff --git a/src/tempo/legacy/client/ChannelOps.test.ts b/src/tempo/legacy/client/ChannelOps.test.ts index 8c4741d2..3cac7f72 100644 --- a/src/tempo/legacy/client/ChannelOps.test.ts +++ b/src/tempo/legacy/client/ChannelOps.test.ts @@ -340,6 +340,27 @@ describe.runIf(isLocalnet)('tryRecoverChannel', () => { expect(result!.opened).toBe(true) expect(result!.escrowContract).toBe(escrow) expect(result!.chainId).toBe(chain.id) + // Channel opened with a zero authorizedSigner: the escrow verifies + // vouchers against the payer, so the recovered authority is the payer. + expect(result!.authorizedSigner.toLowerCase()).toBe(payer.address.toLowerCase()) + }) + + test('recovers the on-chain authorizedSigner when set', async () => { + const authorizedSigner = accounts[4].address + const { channelId: signerChannelId } = await openChannel({ + escrow, + payer, + payee, + token: currency, + deposit: 10_000_000n, + salt: Hex.random(32) as `0x${string}`, + authorizedSigner, + }) + + const result = await tryRecoverChannel(client, escrow, signerChannelId, chain.id) + + expect(result).not.toBeUndefined() + expect(result!.authorizedSigner.toLowerCase()).toBe(authorizedSigner.toLowerCase()) }) test('returns undefined for non-existent channel', async () => { diff --git a/src/tempo/legacy/client/ChannelOps.ts b/src/tempo/legacy/client/ChannelOps.ts index fa216280..f45a2e24 100644 --- a/src/tempo/legacy/client/ChannelOps.ts +++ b/src/tempo/legacy/client/ChannelOps.ts @@ -27,6 +27,8 @@ import { signVoucher } from '../session/Voucher.js' /** Cached channel metadata used by the legacy auto-driving session client. */ export type ChannelEntry = { + /** Voucher authority the channel was opened with. */ + authorizedSigner: Address /** Legacy contract-backed channel ID. */ channelId: Hex.Hex /** Salt used to derive the channel ID. */ @@ -209,6 +211,7 @@ export async function createOpenPayload( return { entry: { + authorizedSigner, channelId, salt, cumulativeAmount: initialAmount, @@ -249,6 +252,10 @@ export async function tryRecoverChannel( if (onChain.deposit > 0n && !onChain.finalized) { return { + // A zero on-chain authorizedSigner means the escrow verifies vouchers + // against the payer, so record the payer as the voucher authority. + authorizedSigner: + BigInt(onChain.authorizedSigner) === 0n ? onChain.payer : onChain.authorizedSigner, channelId, salt: '0x' as Hex.Hex, cumulativeAmount: onChain.settled, diff --git a/src/tempo/legacy/client/Session.test.ts b/src/tempo/legacy/client/Session.test.ts index 6989a4a8..8fe2a834 100644 --- a/src/tempo/legacy/client/Session.test.ts +++ b/src/tempo/legacy/client/Session.test.ts @@ -1,7 +1,16 @@ import { SignatureEnvelope } from 'ox/tempo' -import { type Address, createClient, decodeFunctionData, erc20Abi, type Hex, http } from 'viem' +import { + type Address, + createClient, + custom, + decodeFunctionData, + erc20Abi, + type Hex, + http, +} from 'viem' import { privateKeyToAccount } from 'viem/accounts' import { Account as TempoAccount, Addresses, Transaction, WebCryptoP256 } from 'viem/tempo' +import { tempoModerato } from 'viem/tempo/chains' import { beforeAll, describe, expect, test } from 'vp/test' import { tempoNetwork } from '~test/config.js' import { deployEscrow, openChannel } from '~test/tempo/legacy/session.js' @@ -374,6 +383,98 @@ describe('session (pure)', () => { expect(cred.source).toBe(`did:pkh:eip155:42431:${pureAccount.address}`) }) }) + + describe('auto-manage channel reuse', () => { + const payerAccount = privateKeyToAccount( + '0x59c6995e998f97a5a0044966f09453863d462d2b3f1446a99f0a3d7b5d0f5a0d', + ) + const signerA = TempoAccount.fromSecp256k1( + '0x5de4111afa1a4b94908f83103eb1f1706367c2e68ca870fc3fb9a804cdab365a', + { access: payerAccount }, + ) + const signerB = TempoAccount.fromSecp256k1( + '0x7c852118294e51e653712a81e05800f419141751be58f605c371e15141b007a6', + { access: payerAccount }, + ) + + function makeAutoClient(account: typeof signerA) { + return createClient({ + account, + chain: tempoModerato, + transport: custom({ + async request({ method }: { method: string }) { + if (method === 'eth_chainId') return `0x${tempoModerato.id.toString(16)}` + if (method === 'eth_fillTransaction') + return { + raw: '0x', + tx: { + chainId: `0x${tempoModerato.id.toString(16)}`, + from: payerAccount.address, + gas: '0x5208', + input: '0x', + maxFeePerGas: '0x1', + maxPriorityFeePerGas: '0x1', + nonce: '0x0', + to: escrowAddress, + type: '0x2', + value: '0x0', + }, + } + throw new Error(`unexpected rpc request: ${method}`) + }, + }), + }) + } + + test('reuses the cached channel while the voucher signer is unchanged', async () => { + const method = session({ + getClient: () => makeAutoClient(signerA), + maxDeposit: '10', + }) + + const first = deserializePayload( + await method.createCredential({ challenge: makeChallenge(), context: {} }), + ) + const second = deserializePayload( + await method.createCredential({ challenge: makeChallenge(), context: {} }), + ) + + expect(first.payload.action).toBe('open') + expect(second.payload.action).toBe('voucher') + expect(second.payload.channelId).toBe(first.payload.channelId) + }) + + test('opens a new channel instead of reusing when the authorizedSigner changes', async () => { + let account = signerA + const method = session({ + getClient: () => makeAutoClient(account), + maxDeposit: '10', + }) + + const first = deserializePayload( + await method.createCredential({ challenge: makeChallenge(), context: {} }), + ) + + account = signerB + + const second = deserializePayload( + await method.createCredential({ challenge: makeChallenge(), context: {} }), + ) + + expect(first.payload.action).toBe('open') + if (first.payload.action !== 'open') throw new Error('expected open payload') + expect(first.payload.authorizedSigner?.toLowerCase()).toBe( + signerA.accessKeyAddress.toLowerCase(), + ) + + expect(second.payload.action).toBe('open') + if (second.payload.action !== 'open') throw new Error('expected open payload') + expect(second.payload.authorizedSigner?.toLowerCase()).toBe( + signerB.accessKeyAddress.toLowerCase(), + ) + expect(second.payload.channelId).not.toBe(first.payload.channelId) + }) + }) }) describe.runIf(isLocalnet)('session (on-chain)', () => { diff --git a/src/tempo/legacy/client/Session.ts b/src/tempo/legacy/client/Session.ts index 71c8a1c6..629dd020 100644 --- a/src/tempo/legacy/client/Session.ts +++ b/src/tempo/legacy/client/Session.ts @@ -175,6 +175,16 @@ export function session(parameters: session.Parameters = {}) { } } + // A channel only admits vouchers from the authorizedSigner it was opened + // with, so a cached or recovered entry stops being reusable once the + // resolved signer changes. Fall through to open a fresh channel instead + // of emitting a voucher the escrow would reject. + if ( + entry?.opened && + entry.authorizedSigner.toLowerCase() !== getAccountSignerAddress(voucherSigner).toLowerCase() + ) + entry = undefined + let payload: LegacySessionCredentialPayload if (entry?.opened) {