Skip to content

Add support for continually adding more txns to TransactionWorker, make it more extensible#675

Closed
banool wants to merge 1 commit intomainfrom
banool/simplify-txn-worker
Closed

Add support for continually adding more txns to TransactionWorker, make it more extensible#675
banool wants to merge 1 commit intomainfrom
banool/simplify-txn-worker

Conversation

@banool
Copy link
Contributor

@banool banool commented Apr 4, 2025

Description

The existing behavior of the TransactionWorker is after it runs out of txns to submit, it no longer processes any further txns. This makes the class less useful, and given the existing API design, is quite unexpected, since the push method would still work, the worker wouldn't exit, etc.

This PR makes the worker continually accept new transactions and submit them as they come in.

I also make some other improvements:

  1. Certain methods were needlessly async, making the code a bit confusing, particularly for methods that return promises but don't need to be async.
  2. Deficiencies in other parts of the code have been fixed at the source, e.g. adding a dequeueAll method to AsyncQueue so we don't have to replicate the functionality in processTransactions with a for-loop.
  3. My original goal was to make a version of this that uses the gas station to submit txns instead. With that in mind, there is now a buildSignAndSubmitTransactionPromise method that can be overridden.

Test Plan

See updated tests for the TransactionWorker and AsyncQueue. See also this run of the batch funds example:

pnpm install
pnpm build
cd examples/typescript
pnpm install
pnpm ts-node batch_funds.ts
...
Account sequence number is 20, it should be 20
Account sequence number is 20, it should be 20

Related Links

Checklist

  • Have you ran pnpm fmt?
  • Have you updated the CHANGELOG.md?

@banool banool requested a review from a team as a code owner April 4, 2025 17:49
@banool banool force-pushed the banool/simplify-txn-worker branch from be9282e to e452bd6 Compare April 4, 2025 17:57
Copy link
Contributor

@0xmaayan 0xmaayan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unintentional, probably from using a newer pnpm version. I'll revert it.

this.run();

Promise.all([this.submitNextTransaction(), this.processTransactions()]).catch((error) => {
console.error(`Transaction worker failed: ${error}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not throw the error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should, mb, this is cursor's doing lol.

* @category Transactions
*/
outstandingTransactions = new AsyncQueue<[Promise<PendingTransactionResponse>, bigint]>();
outstandingTransactionHashes = new AsyncQueue<[Promise<string>, bigint]>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we add comments on what are the string, bigint types

@banool banool force-pushed the banool/simplify-txn-worker branch 8 times, most recently from 3e665e7 to d71bda0 Compare April 7, 2025 13:03
@banool banool force-pushed the banool/simplify-txn-worker branch from d71bda0 to 88030ed Compare April 10, 2025 19:15
@banool
Copy link
Contributor Author

banool commented Apr 15, 2025

Deprecating this in favor of #679.

@banool banool closed this Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants