-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the new Soroban RPC simulation method when preparing transactions #77
Changes from 9 commits
79eff07
d15d753
4384b3f
ec786ab
875717a
e62832c
77c81d5
de9a9df
4c98212
e130f98
a853363
5bd4181
3e9f2d0
6b2c183
18314ef
5c2698e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,35 +6,53 @@ import { | |
TransactionBuilder, | ||
xdr, | ||
} from "stellar-base"; | ||
import { SorobanRpc } from "./soroban_rpc"; | ||
|
||
// TODO: Transaction is immutable, so we need to re-build it here. :( | ||
export function assembleTransaction( | ||
raw: Transaction | FeeBumpTransaction, | ||
networkPassphrase: string, | ||
simulated: Array<null | { | ||
footprint: Buffer | string | xdr.LedgerFootprint; | ||
auth: Array<Buffer | string | xdr.ContractAuth>; | ||
}>, | ||
simulation: SorobanRpc.SimulateTransactionResponse, | ||
): Transaction { | ||
if ("innerTransaction" in raw) { | ||
// TODO: Handle feebump transactions | ||
return assembleTransaction( | ||
raw.innerTransaction, | ||
networkPassphrase, | ||
simulated, | ||
simulation, | ||
); | ||
} | ||
|
||
if (simulated.length !== raw.operations.length) { | ||
if ( | ||
raw.operations.length !== 1 || | ||
raw.operations[0].type !== "invokeHostFunction" | ||
) { | ||
throw new Error( | ||
"number of simulated operations not equal to number of transaction operations", | ||
"unsupported operation type, must be only one InvokeHostFunctionOp in the transaction.", | ||
); | ||
} | ||
|
||
const rawInvokeHostFunctionOp: any = raw.operations[0]; | ||
|
||
if ( | ||
!rawInvokeHostFunctionOp.functions || | ||
!simulation.results || | ||
rawInvokeHostFunctionOp.functions.length !== simulation.results.length | ||
) { | ||
throw new Error( | ||
"preflight simulation results do not contain same count of HostFunctions that InvokeHostFunctionOp in the transaction has.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the intricacies of the multi-op host functions yet: why is the length enforced to be 1 above, but that isn't enforced here? I guess it's implicit, but I just want to make sure. Related: is there no way to simulate transactions with multiple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dmkozh , can you confirm with the latest changes for multi-function InvokeHostFunctionOp, the limitation for a single InvokeHostFunctionOp in Tx is still being enforced? I thought have seen that mentioned in chat and was assuming that limitation on validation of tx here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ohhhh yeah sorry, you're right 🤦 one op per tx, but the op can have many fns. got it! 😆 too many layers 🍰 |
||
); | ||
} | ||
|
||
// TODO: Figure out a cleaner way to clone this transaction. | ||
const source = new Account(raw.source, `${parseInt(raw.sequence, 10) - 1}`); | ||
const txn = new TransactionBuilder(source, { | ||
fee: raw.fee, | ||
const classicFeeNum = parseInt(raw.fee, 10) || 0; | ||
const minResourceFeeNum = parseInt(simulation.minResourceFee, 10) || 0; | ||
const txnBuilder = new TransactionBuilder(source, { | ||
// automatically update the 'classic' tx fee if min resource fees from simulation response | ||
// surpass that value. | ||
// 'classic' fees are measured as the product of tx.fee * 'number of operations', In soroban contract tx, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. classic fees are There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, on classic tx's that would be the way classic fees would result, with the limitation of only one operation in the soroban tx, have reduced to just classic fees = classicFeeNum(the parsed numeric value of user specified tx.fee). |
||
// there can only be single operation in the tx, so can safely make simplification of classic fee = tx.fee. | ||
fee: Math.max(classicFeeNum + minResourceFeeNum, classicFeeNum).toString(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wait, when will this ever return the second case of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the second argument to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, good point, max is un-needed at this point, the incoming tx.fee has really become the 'suggested inclusion fee' when using I think we can only append minResourceFee to what user provides on incoming tx.fee and return the sum as the updated tx.fee on the returned transaction from |
||
memo: raw.memo, | ||
networkPassphrase, | ||
timebounds: raw.timeBounds, | ||
|
@@ -44,35 +62,47 @@ export function assembleTransaction( | |
minAccountSequenceLedgerGap: raw.minAccountSequenceLedgerGap, | ||
extraSigners: raw.extraSigners, | ||
}); | ||
for (let i = 0; i < raw.operations.length; i++) { | ||
const rawOp = raw.operations[i]; | ||
if ("function" in rawOp) { | ||
const sim = simulated[i]; | ||
if (!sim) { | ||
throw new Error("missing simulated operation"); | ||
} | ||
let footprint = sim.footprint ?? rawOp.footprint; | ||
if (!(footprint instanceof xdr.LedgerFootprint)) { | ||
footprint = xdr.LedgerFootprint.fromXDR(footprint.toString(), "base64"); | ||
} | ||
const auth = (sim.auth ?? rawOp.auth).map((a) => | ||
a instanceof xdr.ContractAuth | ||
? a | ||
: xdr.ContractAuth.fromXDR(a.toString(), "base64"), | ||
); | ||
// TODO: Figure out a cleaner way to clone these operations | ||
txn.addOperation( | ||
Operation.invokeHostFunction({ | ||
function: rawOp.function, | ||
parameters: rawOp.parameters, | ||
footprint, | ||
auth, | ||
}), | ||
); | ||
} else { | ||
// TODO: Handle this. | ||
throw new Error("Unsupported operation type"); | ||
} | ||
|
||
// apply the pre-built Auth from simulation onto each Tx/Op/HostFunction | ||
// invocation | ||
const authDecoratedHostFunctions = simulation.results.map( | ||
(functionSimulationResult, i) => { | ||
const hostFn: xdr.HostFunction = rawInvokeHostFunctionOp.functions[i]; | ||
hostFn.auth(buildContractAuth(functionSimulationResult.auth)); | ||
return hostFn; | ||
}, | ||
); | ||
|
||
txnBuilder.addOperation( | ||
Operation.invokeHostFunctions({ | ||
functions: authDecoratedHostFunctions, | ||
}), | ||
); | ||
|
||
// apply the pre-built Soroban Tx Ext from simulation onto the Tx | ||
txnBuilder.setExt(buildExt(simulation.transactionData)); | ||
|
||
return txnBuilder.build(); | ||
} | ||
|
||
function buildExt(sorobanTxDataStr: string) { | ||
const sorobanTxData: xdr.SorobanTransactionData = xdr.SorobanTransactionData.fromXDR( | ||
sreuland marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sorobanTxDataStr, | ||
"base64", | ||
); | ||
|
||
// TODO - remove this workaround to use constructor on js-xdr union | ||
// and use the typescript generated static factory method once fixed | ||
// https://github.com/stellar/dts-xdr/issues/5 | ||
// @ts-ignore | ||
const txExt = new xdr.TransactionExt(1, sorobanTxData); | ||
return txExt; | ||
} | ||
sreuland marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
function buildContractAuth(auths: string[]): xdr.ContractAuth[] { | ||
const contractAuths: xdr.ContractAuth[] = []; | ||
for (const authStr of auths) { | ||
contractAuths.push(xdr.ContractAuth.fromXDR(authStr, "base64")); | ||
} | ||
return txn.build(); | ||
return contractAuths; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this to
v9.0.0-soroban.1
when it's out, for now using this slightly newer stellar-base that has tweaks on txbuilder - js-stellar-base#604There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just going to unresolve this so you don't forget after you get approval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's out! https://www.npmjs.com/package/stellar-base/v/9.0.0-soroban.1