feat: adds LocalPendingTransactionStorage implementation #4408
Conversation
…tests Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
simzzz
changed the title
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## feat/transaction-pool #4408 +/- ##
========================================================
Coverage ? 95.70%
========================================================
Files ? 123
Lines ? 20107
Branches ? 1756
========================================================
Hits ? 19244
Misses ? 838
Partials ? 25
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
| } | ||
|
|
||
| // Also remove from transaction data map | ||
| this.transactionData.delete(txHash); |
There was a problem hiding this comment.
When does this.transactionData have a record of txHash? I don't see the txHash got added to this.transactionData at any point, right?
| * @param addr - The account address to query | ||
| * @returns Promise resolving to the number of pending transactions | ||
| */ | ||
| async getList(addr: string): Promise<number> { |
There was a problem hiding this comment.
This is kinda misleading in a sense. getList() sounds like it's getting a list of the actual values, while this method is returning the size of the list. Should we rename it to some else more descriptive? Not a big deal and no need to address in this PR.
| async addToList(addr: string, txHash: string, expectedPending: number): Promise<AddToListResult> { | ||
| const currentCount = await this.getList(addr); | ||
|
|
||
| // Check if the current count matches expectations (optimistic concurrency control) |
There was a problem hiding this comment.
Is the purpose of optimistic concurrency control to prevent race conditioning? If so I think we should add more context for this. Can address in upcoming PRs
Description
This PR adds implementation for LocalPendingTransactionStorage following the PendingTransactionStorage + relevant tests
Related issue(s)
Closes #4391
Testing Guide
Changes from original design (optional)
The interface has been slightly modified to pass the txHash to addToList
Additional work needed (optional)
N/A
Checklist