From 64daed143260cdf2876e5d0651ab970c91813d70 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 31 Jan 2024 22:02:03 -0700 Subject: [PATCH 1/6] adds checks for address credentials auth entries, auth entries contractId mismatch, and sub invocations to token payment simulation --- src/helper/error.ts | 7 +++++++ src/helper/horizon-rpc.ts | 2 +- src/route/index.ts | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/helper/error.ts diff --git a/src/helper/error.ts b/src/helper/error.ts new file mode 100644 index 0000000..6c5fb66 --- /dev/null +++ b/src/helper/error.ts @@ -0,0 +1,7 @@ +export const ERROR = { + ACCOUNT_NOT_SOURCE: + "transfer contains authorization entry for a different account", + FOREIGN_CONTRACT_AUTH: + "transfer contains authorization entry for another contract", + SUB_INVOCATIONS: "transfer contains sub-invocations to another contract", +}; diff --git a/src/helper/horizon-rpc.ts b/src/helper/horizon-rpc.ts index 91b7a4f..d5522db 100644 --- a/src/helper/horizon-rpc.ts +++ b/src/helper/horizon-rpc.ts @@ -248,7 +248,7 @@ export const submitTransaction = async ( error: null, }; } catch (e: any) { - if (e.response.status === 504) { + if (e.response?.status === 504) { // in case of 504, keep retrying this tx until submission succeeds or we get a different error // https://developers.stellar.org/api/errors/http-status-codes/horizon-specific/timeout // https://developers.stellar.org/docs/encyclopedia/error-handling diff --git a/src/route/index.ts b/src/route/index.ts index 4c67935..cff5daf 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -25,8 +25,10 @@ import { Transaction, TransactionBuilder, XdrLargeInt, + xdr, } from "stellar-sdk"; import { buildTransfer, simulateTx } from "../helper/soroban-rpc"; +import { ERROR } from "../helper/error"; const API_VERSION = "v1"; @@ -549,17 +551,48 @@ export async function initApiServer( const simulationResponse = (await server.simulateTransaction( tx )) as SorobanRpc.Api.SimulateTransactionSuccessResponse; + const preparedTransaction = SorobanRpc.assembleTransaction( tx, simulationResponse ); + + const built = preparedTransaction.build(); + const sorobanOp = built + .operations[0] as Operation.InvokeHostFunction; + const auths = sorobanOp.auth || []; + + for (const auth of auths) { + if ( + auth.credentials().switch() !== + xdr.SorobanCredentialsType.sorobanCredentialsSourceAccount() + ) { + throw new Error(ERROR.ACCOUNT_NOT_SOURCE); + } + if ( + auth.credentials().switch() === + xdr.SorobanCredentialsType.sorobanCredentialsAddress() && + auth + .credentials() + ?.address() + ?.address() + .contractId() + .toString() !== address + ) { + throw new Error(ERROR.FOREIGN_CONTRACT_AUTH); + } + if (auth.rootInvocation().subInvocations().length) { + throw new Error(ERROR.SUB_INVOCATIONS); + } + } + const data = { simulationResponse, - preparedTransaction: preparedTransaction.build().toXDR(), + preparedTransaction: built.toXDR(), }; reply.code(200).send(data); } catch (error) { - reply.code(400).send(JSON.stringify(error)); + reply.code(400).send(error); } }, }); From 64a286903bcda76dddaa08703d0f773af561dcca Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Thu, 1 Feb 2024 12:18:51 -0700 Subject: [PATCH 2/6] capitalize token sim error strings --- src/helper/error.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/helper/error.ts b/src/helper/error.ts index 6c5fb66..221b0bc 100644 --- a/src/helper/error.ts +++ b/src/helper/error.ts @@ -1,7 +1,7 @@ export const ERROR = { ACCOUNT_NOT_SOURCE: - "transfer contains authorization entry for a different account", + "Transfer contains authorization entry for a different account", FOREIGN_CONTRACT_AUTH: - "transfer contains authorization entry for another contract", - SUB_INVOCATIONS: "transfer contains sub-invocations to another contract", + "Transfer contains authorization entry for another contract", + SUB_INVOCATIONS: "Transfer contains sub-invocations to another contract", }; From 2b943fbac28c06eb9add751f3477c109aadcd89f Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Thu, 1 Feb 2024 12:45:21 -0700 Subject: [PATCH 3/6] remove check for sorobanCredentialsAddress, already covered by the previous check --- src/route/index.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/route/index.ts b/src/route/index.ts index cff5daf..5ee80d6 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -569,18 +569,7 @@ export async function initApiServer( ) { throw new Error(ERROR.ACCOUNT_NOT_SOURCE); } - if ( - auth.credentials().switch() === - xdr.SorobanCredentialsType.sorobanCredentialsAddress() && - auth - .credentials() - ?.address() - ?.address() - .contractId() - .toString() !== address - ) { - throw new Error(ERROR.FOREIGN_CONTRACT_AUTH); - } + if (auth.rootInvocation().subInvocations().length) { throw new Error(ERROR.SUB_INVOCATIONS); } From 3272c3c01df00ef3db048298a44055acca7e0b97 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Mon, 5 Feb 2024 15:26:47 -0700 Subject: [PATCH 4/6] adds explicit check for op type in token sim, updates jwt expired error from mercury, removes unused token sim error --- src/helper/error.ts | 2 -- src/route/index.ts | 32 ++++++++++++++++++-------------- src/service/mercury/index.ts | 4 ++-- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/helper/error.ts b/src/helper/error.ts index 221b0bc..095773f 100644 --- a/src/helper/error.ts +++ b/src/helper/error.ts @@ -1,7 +1,5 @@ export const ERROR = { ACCOUNT_NOT_SOURCE: "Transfer contains authorization entry for a different account", - FOREIGN_CONTRACT_AUTH: - "Transfer contains authorization entry for another contract", SUB_INVOCATIONS: "Transfer contains sub-invocations to another contract", }; diff --git a/src/route/index.ts b/src/route/index.ts index 445b59e..8a5197c 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -554,20 +554,24 @@ export async function initApiServer( ); const built = preparedTransaction.build(); - const sorobanOp = built - .operations[0] as Operation.InvokeHostFunction; - const auths = sorobanOp.auth || []; - - for (const auth of auths) { - if ( - auth.credentials().switch() !== - xdr.SorobanCredentialsType.sorobanCredentialsSourceAccount() - ) { - throw new Error(ERROR.ACCOUNT_NOT_SOURCE); - } - - if (auth.rootInvocation().subInvocations().length) { - throw new Error(ERROR.SUB_INVOCATIONS); + switch (built.operations[0].type) { + case "invokeHostFunction": { + const sorobanOp = built + .operations[0] as Operation.InvokeHostFunction; + const auths = sorobanOp.auth || []; + + for (const auth of auths) { + if ( + auth.credentials().switch() !== + xdr.SorobanCredentialsType.sorobanCredentialsSourceAccount() + ) { + throw new Error(ERROR.ACCOUNT_NOT_SOURCE); + } + + if (auth.rootInvocation().subInvocations().length) { + throw new Error(ERROR.SUB_INVOCATIONS); + } + } } } diff --git a/src/service/mercury/index.ts b/src/service/mercury/index.ts index 2bc4273..a3a654f 100644 --- a/src/service/mercury/index.ts +++ b/src/service/mercury/index.ts @@ -34,7 +34,7 @@ enum NETWORK_URLS { } const ERROR_MESSAGES = { - JWT_EXPIRED: "jwt expired", + JWT_EXPIRED: "1_kJdMBB7ytvgRIqF1clh2iz2iI", }; function getGraphQlError(error?: CombinedError) { @@ -179,7 +179,7 @@ export class MercuryClient { } catch (error: unknown) { // renew and retry 0n 401, otherwise throw the error back up to the caller if (error instanceof Error) { - if (error.message === ERROR_MESSAGES.JWT_EXPIRED) { + if (error.message.includes(ERROR_MESSAGES.JWT_EXPIRED)) { await this.renewMercuryToken(); this.logger.info("renewed expired jwt"); return await method(); From 939623c00cf2f71612054a14e7bc59e4c58d9be0 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Wed, 7 Feb 2024 14:53:14 -0700 Subject: [PATCH 5/6] reorganized token details getter to continue on error to get a tokens details --- src/service/mercury/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/service/mercury/index.ts b/src/service/mercury/index.ts index a3a654f..6bfd40b 100644 --- a/src/service/mercury/index.ts +++ b/src/service/mercury/index.ts @@ -518,9 +518,10 @@ export class MercuryClient { ) => { const balances = []; const balanceMap = {} as Record; - try { - const server = await getServer(network, customSorobanRpcUrl); - for (const id of contractIds) { + + const server = await getServer(network, customSorobanRpcUrl); + for (const id of contractIds) { + try { const builder = await getTxBuilder(pubKey, network, server); const params = [new Address(pubKey).toScVal()]; const balance = await getTokenBalance(id, params, server, builder); @@ -530,11 +531,12 @@ export class MercuryClient { balance, ...tokenDetails, }); + } catch (error) { + this.logger.error(error); + continue; } - } catch (error) { - this.logger.error(error); - return balanceMap; } + for (const balance of balances) { balanceMap[`${balance.symbol}:${balance.id}`] = { token: { From 99d3e331d15805a305df97d1a63e24b8ccebb7e6 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Thu, 8 Feb 2024 09:52:50 -0700 Subject: [PATCH 6/6] tweak error display text for auth sub invocations --- src/helper/error.ts | 3 ++- src/route/index.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/helper/error.ts b/src/helper/error.ts index 095773f..4d564fa 100644 --- a/src/helper/error.ts +++ b/src/helper/error.ts @@ -1,5 +1,6 @@ export const ERROR = { ACCOUNT_NOT_SOURCE: "Transfer contains authorization entry for a different account", - SUB_INVOCATIONS: "Transfer contains sub-invocations to another contract", + AUTH_SUB_INVOCATIONS: + "Transfer authorizes sub-invocations to another contract", }; diff --git a/src/route/index.ts b/src/route/index.ts index 8a5197c..19d470c 100644 --- a/src/route/index.ts +++ b/src/route/index.ts @@ -569,7 +569,7 @@ export async function initApiServer( } if (auth.rootInvocation().subInvocations().length) { - throw new Error(ERROR.SUB_INVOCATIONS); + throw new Error(ERROR.AUTH_SUB_INVOCATIONS); } } }