Skip to content

Commit

Permalink
fix: don't make assumptions about expiration
Browse files Browse the repository at this point in the history
The `signedAuthEntry` assembled in `submitTx` must have a
`signatureExpirationLedger` that matches the one initially created in
`invoke`.

Given that significant time may have passed between the initial `invoke`
call and the `submitTx` call, we cannot assume that the state expiration
has remained unchanged. It's safer to use the initial value that was
created in `invoke`.

The most straightforward way to accomplish this was to return
`xdr.HashIdPreimage` objects, rather than hashes of them. This requires
some more work on the part of the consumer: they now need to import
`hash` and create `payload` themself, as well as keeping the original
`signHere` around to send along to the `submitTx` call.
  • Loading branch information
chadoh committed Oct 26, 2023
1 parent adf54a6 commit f2453ca
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 37 deletions.
4 changes: 2 additions & 2 deletions cmd/crates/soroban-spec-typescript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ fn generate_class(fns: &[Entry], spec: &[ScSpecEntry]) -> String {
{spec}
]);
}}
submitTx(txXdr: XDR_BASE64, allSigned: PubKeyToAuthEntries, secondsToWait = 10): Promise<SubmitResponse> {{
return submitTx({{ txXdr, allSigned, secondsToWait, ...this.options }});
submitTx(txXdr: XDR_BASE64, signHere: PubKeyToPreimagesUnsigned, allSigned: PubKeyToPreimagesSigned, secondsToWait = 10): Promise<SubmitResponse> {{
return submitTx({{ txXdr, signHere, allSigned, secondsToWait, ...this.options }});
}}
{methods}
}}"#,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import * as SorobanClient from 'soroban-client';
import { ContractSpec, Address } from 'soroban-client';
import { Buffer } from "buffer";
import { invoke, submitTx } from './invoke.js';
import type { PubKeyToAuthEntries, SubmitResponse } from './invoke.js';
import type { ResponseTypes, Wallet, ClassOptions, XDR_BASE64 } from './method-options.js'
import type { PubKeyToPreimagesUnsigned, PubKeyToPreimagesSigned, SubmitResponse } from './invoke.js';
import type { ResponseTypes, Wallet, ClassOptions, XDR_BASE64 } from './method-options.js';

export * from './invoke.js'
export * from './method-options.js'
export * from './invoke.js';
export * from './method-options.js';

export { hash } from 'soroban-client';

export type u32 = number;
export type i32 = number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ type InvokeArgs<R extends ResponseTypes, T = string> = MethodOptions<R> &
parseResultXdr: (xdr: string | xdr.ScVal) => T;
};

export type PubKeyToAuthEntries = Map<string, Buffer[]>;
export type PubKeyToPreimagesUnsigned = Map<string, xdr.HashIdPreimage[]>;
export type PubKeyToPreimagesSigned = Map<string, Buffer[]>;

export interface InvokeGeneric<T> {
txUnsigned: XDR_BASE64,
Expand All @@ -71,7 +72,7 @@ export interface InvokeGeneric<T> {
result?: T,
}

export type InvokeSimulation = Pick<InvokeGeneric<undefined>, 'txUnsigned' | 'simulation'> & { signHere?: PubKeyToAuthEntries };
export type InvokeSimulation = Pick<InvokeGeneric<undefined>, 'txUnsigned' | 'simulation'> & { signHere?: PubKeyToPreimagesUnsigned };

/**
* might be a read/view call, which means `txSigned`, `sendTx` and `getTx` will all be unnecessary, hence keeping them as optional
Expand Down Expand Up @@ -144,14 +145,24 @@ export async function invoke<R extends ResponseTypes, T = string>({
throw new ExpiredStateError(`You need to restore some contract state before you can invoke this method. ${JSON.stringify(simulation, null, 2)}`);
}

const signHere = await getSignHere(txUnsigned, contractId, server, networkPassphrase);
const signHere = await getSignHere(txUnsigned, contractId, server, networkPassphrase)

if (signHere && responseType !== "simulated") {
throw new NeedsMoreSignaturesError(
"Transaction has Auth Entries that need to be signed by someone other than the invoker! " +
"Use `invoke({ responseType: 'simulation' })` to get a `signHere` object, which will contain public-key indexed list of auth entries. Here's a list of the public keys of the accounts that will need to sign these auth entries:\n\n" +
`- ${Object.keys(signHere).join('\n- ')}` +
"\n\nOnce you update `signHere` with signatures (or construct a new one), you can use `submitTx(unsignedTx, signHere)` to submit the transaction."
`Transaction has Auth Entries that need to be signed by someone other than the invoker! ` +
`Use \`invoke({ responseType: 'simulation' })\` to get a \`signHere\` object, ` +
`which will contain public-key indexed Map of HashIdPreimages for that pubkey-holder to sign. ` +
`Use \`signHere\` to create a similar Map with signatures, kind of like this:\n
const allSigned = new Map()
for (const [pk, preimages] of signHere) {
for (const preimage of preimages) {
const payload = hash(preimage.toXDR())
const signature = await sign(payload)
allSigned.set(pk, [...allSigned.get(pk) ?? [], signature])
}
}\n\n` +
`Then you can submit the transaction using \`submitTx(unsignedTx, signHere, allSigned)\`.\n\n` +
`You can import \`hash\` from this library, but you will need to bring your own \`sign\` function!`
)
}

Expand Down Expand Up @@ -397,7 +408,7 @@ export async function getSignHere(
contractId: string,
server: Server,
networkPassphrase: string,
): Promise<PubKeyToAuthEntries | undefined> {
): Promise<PubKeyToPreimagesUnsigned | undefined> {
// We expect that any transaction constructed by these libraries has a
// single operation, which is an InvokeHostFunction operation. The host
// function being invoked is the contract method call.
Expand All @@ -414,7 +425,7 @@ export async function getSignHere(
xdr.SorobanCredentialsType.sorobanCredentialsAddress()
)

let signHere: undefined | PubKeyToAuthEntries = undefined
let signHere: undefined | PubKeyToPreimagesUnsigned = undefined

if (needSigningBySomeoneOtherThanInvoker.length > 0) {
// Set auth entry to expire when contract data expires.
Expand All @@ -439,11 +450,10 @@ export async function getSignHere(
signatureExpirationLedger,
}),
)
const payload = hash(preimage.toXDR())
map.set(pk, [...map.get(pk) ?? [], payload])
map.set(pk, [...map.get(pk) ?? [], preimage])
return map
},
new Map() as PubKeyToAuthEntries
new Map() as PubKeyToPreimagesUnsigned
)
}

Expand All @@ -452,20 +462,22 @@ export async function getSignHere(

type SubmitArgs = ClassOptions & Pick<MethodOptions<undefined>, 'secondsToWait'> & {
txXdr: XDR_BASE64,
allSigned: PubKeyToAuthEntries,
};
signHere: PubKeyToPreimagesUnsigned,
allSigned: PubKeyToPreimagesSigned,
}

/**
* Calling `invoke` might require only a simulation, in the case of view calls, in which case `sendTransaction` is never called.
* `submitTx` always calls `sendTransaction`, so we can mark `sendTransactionResponse` as always being present.
*/
export type SubmitResponse = InvokeFull & {
sendTransactionResponse: SendTx,
};
}

export async function submitTx(args: SubmitArgs): Promise<SubmitResponse>;
export async function submitTx({
txXdr,
signHere,
allSigned,
secondsToWait = 10,
contractId,
Expand All @@ -487,13 +499,19 @@ export async function submitTx({
}

txUnsigned = txUnsigned as Tx
const signHere = await getSignHere(txUnsigned, contractId, server, networkPassphrase)

if (!signHere) {
const newSignHere = await getSignHere(txUnsigned, contractId, server, networkPassphrase)

if (!newSignHere) {
throw new Error(`transaction does not need to be signed by anyone other than the invoker; no need to use 'submitTx'`)
}

for (const [pk, entry] of signHere) {
if (!newSignHere.has(pk) && newSignHere.get(pk)!.length !== entry.length) {
throw new Error(
`'signHere' has an unexpected format; the transaction may not require the same signature shape anymore.`
)
}
if (!allSigned.has(pk)) throw new Error(`missing signatures for ${pk}`)
if (entry.length !== allSigned.get(pk)!.length) {
throw new Error(
Expand All @@ -503,6 +521,11 @@ export async function submitTx({
)
}
}
for (const [pk] of newSignHere) {
if (!signHere.has(pk)) {
throw new Error(`'allSigned' contains signatures that may no longer be required by the transaction for publicKey=${pk}`)
}
}
for (const [pk] of allSigned) {
if (!signHere.has(pk)) {
throw new Error(`'allSigned' contains unnecessary signatures for publicKey=${pk}`)
Expand All @@ -528,7 +551,8 @@ export async function submitTx({
const pk = StrKey.encodeEd25519PublicKey(
entry.credentials().address().address().accountId().ed25519()
)
const payload = signHere.get(pk)?.shift()!
const preimage = signHere.get(pk)?.shift()!
const payload = hash(preimage.toXDR())
const signature = allSigned.get(pk)?.shift()!
if (!Keypair.fromPublicKey(pk).verify(payload, signature)) {
throw new Error(
Expand Down Expand Up @@ -558,14 +582,9 @@ export async function submitTx({
const addrAuth = entry.credentials().address()
addrAuth.signature(xdr.ScVal.scvVec([sigScVal]))

// also need to set the expiration to match the one in signature;
// let's assume persistent storage expiration hasn't changed
const signatureExpirationLedger = await getStorageExpiration(
contractId,
'persistent',
server
addrAuth.signatureExpirationLedger(
preimage.sorobanAuthorization().signatureExpirationLedger()
)
addrAuth.signatureExpirationLedger(signatureExpirationLedger)

signedAuthEntries.push(entry)
}
Expand Down
13 changes: 7 additions & 6 deletions cmd/crates/soroban-spec-typescript/ts-tests/src/test-swap.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import test from "ava"
import { wallet, rpcUrl, root, alice, networkPassphrase } from "./util.js"
import { Contract as Token } from "token"
import { Contract as Swap, networks, NeedsMoreSignaturesError } from "test-swap"
import { Contract as Swap, networks, NeedsMoreSignaturesError, hash } from "test-swap"
import fs from "node:fs"

const tokenAId = fs.readFileSync(new URL("../contract-id-token-a.txt", import.meta.url), "utf8").trim()
Expand Down Expand Up @@ -73,17 +73,18 @@ test('root swaps alice 10 A for 1 B', async t => {
}

const signatures: Map<string, Buffer[]> = new Map()
for (const [pk, entries] of signHere) {
for (const [pk, preimages] of signHere) {
t.is(pk, alice.keypair.publicKey())
t.is(entries.length, 1)
t.is(preimages.length, 1)

for (const entry of entries) {
const signature = alice.keypair.sign(entry)
for (const preimage of preimages) {
const payload = hash(preimage.toXDR())
const signature = alice.keypair.sign(payload)
signatures.set(pk, [...signatures.get(pk) ?? [], signature])
}
}

const result = await swap.submitTx(txUnsigned, signatures)
const result = await swap.submitTx(txUnsigned, signHere, signatures)

t.truthy(result.sendTransactionResponse, `tx failed: ${JSON.stringify(result)}`)
t.true(result.sendTransactionResponse.status === 'PENDING', `tx failed: ${JSON.stringify(result)}`)
Expand Down

0 comments on commit f2453ca

Please sign in to comment.