STREAM-1451: solana reliable tx execution #158

Yolley · 2024-04-05T07:08:56Z

simulate and send transaction separately
rebroadcast transaction until we're sure it hasn't or has landed
switch to VersionedTransaction

LukaStreamflow · 2024-04-05T10:57:10Z

packages/common/solana/utils.ts

+  invoker: Keypair | SignerWalletAdapter,
+  tx: T,
+): Promise<T> {
+  let signedTx: T;


Suggested change

let signedTx: T;

if (isSignerWallet(invoker)) {

return await invoker.signTransaction(tx);

}

if (isTransactionVersioned(tx)) {

return tx.sign([invoker]);

}

return tx.partialSign(invoker);

I find it more readable

LukaStreamflow · 2024-04-05T10:58:37Z

packages/common/solana/utils.ts

+  for (let i = 0; i < 3; i++) {
+    let res: RpcResponseAndContext<SimulatedTransactionResponse>;
+    if (isTransactionVersioned(tx)) {
+      res = await connection.simulateTransaction(tx);


Shouldn't be the same as else? :)

I had the same thought, but looks like method overloading in TS works differently and it's incapable of handling this correctly. Therefore, this will produce an error because of polymorphic input. I guess there is some way of casting tx correctly, but it won't make this place easier to read 😔

What's more likely btw, solana.web3 produces incomplete type declaration for this function

LukaStreamflow · 2024-04-05T10:59:11Z

packages/common/solana/utils.ts

+      res = await connection.simulateTransaction(tx);
+    }
+    if (res.value.err) {
+      const errMessage = JSON.stringify(res.value.err);


Nit: Let's be fancy and specify number of retries as constant, instead of "3".
Will be more readable also

LukaStreamflow · 2024-04-05T10:59:36Z

packages/common/solana/utils.ts

+    throw Error("Error with transaction parameters.");
+  }
+
+  for (let i = 0; i < 3; i++) {


I'd put whole simulation stuff into function

LukaStreamflow · 2024-04-05T11:00:39Z

packages/common/solana/utils.ts

+  let blockheight = await connection.getBlockHeight(commitment);
+  let transactionSent = false;
+  const rawTransaction = tx.serialize();
+  while (blockheight < hash.lastValidBlockHeight) {


I propose extracting this in a function also

LukaStreamflow · 2024-04-05T11:01:48Z

packages/common/solana/utils.ts

 }

 /**
 * Confirms and validates transaction success once
 * @param connection - Solana client connection
 * @param signature - Transaction signature
+ * @param passError - return status even if tx failed


I find naming a bit strange, what should passError mean? Could be just successful?

Or maybe better, ignoreError, if we are just printing it ant not throwing

LukaStreamflow · 2024-04-05T11:03:13Z

packages/common/solana/utils.ts

  );
-  return signAndExecuteTransaction(connection, invoker, tx, hash);
+  return signAndExecuteTransaction(connection, invoker, tx, { hash, context, commitment });


We should definitely split utils into meaningful files :) I guess for one of next PRs

LukaStreamflow · 2024-04-05T11:03:44Z

packages/distributor/solana/client.ts

@@ -230,8 +238,12 @@ export default class SolanaDistributorClient {

    ixs.push(clawback(accounts, this.programId));

-    const { tx, hash } = await prepareTransaction(this.connection, ixs, invoker.publicKey);
-    const signature = await wrappedSignAndExecuteTransaction(this.connection, invoker, tx, hash);
+    const { tx, hash, context } = await prepareTransaction(this.connection, ixs, invoker.publicKey);


3 times copy pasta

LukaStreamflow · 2024-04-05T11:04:20Z

packages/stream/solana/StreamClient.ts

@@ -799,8 +829,12 @@ export default class SolanaStreamClient extends BaseStreamClient {
   */
  public async update(data: IUpdateData, extParams: IInteractStreamSolanaExt): Promise<ITransactionResult> {
    const ixs = await this.prepareUpdateInstructions(data, extParams);
-    const { tx, hash } = await prepareTransaction(this.connection, ixs, extParams.invoker.publicKey);
-    const signature = await signAndExecuteTransaction(this.connection, extParams.invoker, tx, hash);
+    const { tx, hash, context } = await prepareTransaction(this.connection, ixs, extParams.invoker.publicKey);


3 times copy pasta

LukaStreamflow · 2024-04-05T11:04:40Z

packages/stream/solana/StreamClient.ts

@@ -866,7 +900,8 @@ export default class SolanaStreamClient extends BaseStreamClient {
  }

  public extractErrorCode(err: Error): string | null {
-    return extractSolanaErrorCode(err.toString() ?? "Unknown error!");
+    const logs = "logs" in err && Array.isArray(err.logs) ? err.logs : undefined;
+    return extractSolanaErrorCode(err.toString() ?? "Unknown error!", logs);


RolginRoman · 2024-04-05T10:37:09Z

packages/common/solana/utils.ts

 * @returns Transaction Status
 */
 export async function confirmAndEnsureTransaction(
  connection: Connection,
  signature: string,
+  passError?: boolean,


I'd prefer more direct naming as throwError as it points to clearer understanding of its usage

imo passError might mean that either error is going to be passed as value or error is going go to be skipped (as pass ~= skip in this case)

RolginRoman · 2024-04-05T10:39:15Z

packages/common/solana/utils.ts

-  }
+  const { value: hash, context } = await connection.getLatestBlockhashAndContext(commitment);
+  const messageV0 = new TransactionMessage({
+    payerKey: payer!,


Im wondering if it is a valid case if the payer is falsy.

should we fail early in this case? should we disallow this case on type layer?

RolginRoman · 2024-04-05T10:58:21Z

packages/common/solana/utils.ts

+        throw new SendTransactionError("failed to simulate transaction: " + errMessage, res.value.logs || undefined);
+      }
+    }
+    break;


is it unused? it is wrapping instruction already

RolginRoman · 2024-04-05T10:58:51Z

packages/common/solana/utils.ts

+    throw Error("Error with transaction parameters.");
+  }
+
+  for (let i = 0; i < 3; i++) {


I've read the js doc here :d
but why 3?

I'd say it should be a time-constrained retry ?

RolginRoman · 2024-04-05T11:02:32Z

packages/common/solana/utils.ts

+      }
+      throw e;
+    }
+    await sleep(500);


why should it sleep every time ? May it skip sleep if transactionSent === true?

We rebroadcasting here and sending transaction over and over again

RolginRoman · 2024-04-05T11:09:09Z

packages/common/solana/utils.ts

+      transactionSent = true;
+    } catch (e) {
+      if (
+        transactionSent ||


if transactionSent == true, why we continue aka execute another loop cycle aka sendRawTransaction again?

Because we are rebroadcasting, please read the comments of the function

RolginRoman

We could convert comments into follow-up issues/tasks for implementing. But since it is urgent (as always), we can try this V in the app

github-actions · 2024-04-05T12:54:42Z

https://github.com/streamflow-finance/js-sdk/releases/tag/v6.1.0 is released 🎉

Yolley added 3 commits April 5, 2024 15:42

STREAM-1451: solana reliable tx execution

bb6aeb0

fix error parsing

b94c6a9

bump

fe3fbff

Yolley requested review from RolginRoman and LukaStreamflow and removed request for RolginRoman April 5, 2024 07:09

Yolley added 4 commits April 5, 2024 16:15

optimise confirmation a bit, add comments with explanation

40ed2db

fix comment

887d737

event more bullet proof lol

992cf9e

lint

c7afa8c

LukaStreamflow reviewed Apr 5, 2024

View reviewed changes

RolginRoman reviewed Apr 5, 2024

View reviewed changes

RolginRoman approved these changes Apr 5, 2024

View reviewed changes

RolginRoman merged commit 170947f into master Apr 5, 2024
2 checks passed

RolginRoman deleted the oleg/feature/STREAM-1451_reliable_tx_execution branch April 5, 2024 12:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STREAM-1451: solana reliable tx execution #158

STREAM-1451: solana reliable tx execution #158

Yolley commented Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

RolginRoman Apr 5, 2024

RolginRoman Apr 5, 2024 •

edited

Loading

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

LukaStreamflow Apr 5, 2024

RolginRoman Apr 5, 2024

RolginRoman Apr 5, 2024

RolginRoman Apr 5, 2024

RolginRoman Apr 5, 2024

RolginRoman Apr 5, 2024

Yolley Apr 5, 2024

RolginRoman Apr 5, 2024

Yolley Apr 5, 2024

RolginRoman left a comment

github-actions bot commented Apr 5, 2024

-  let signedTx: T;
+  if (isSignerWallet(invoker)) {
+    return await invoker.signTransaction(tx);
+  }
+  if (isTransactionVersioned(tx)) {
+    return tx.sign([invoker]);
+  }
+  return tx.partialSign(invoker);

STREAM-1451: solana reliable tx execution #158

STREAM-1451: solana reliable tx execution #158

Conversation

Yolley commented Apr 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RolginRoman Apr 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RolginRoman left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2024

RolginRoman Apr 5, 2024 •

edited

Loading