feat: process tx after push #813

r4mmer · 2025-02-04T16:33:21Z

Motivation

The process to send a transaction from the wallet-lib includes the steps:

Choose inputs, outputs and metadata that will define the transaction.
Create a Transaction instance and calculate the metadata for mining.
Send the tx to the tx-mining-service and get the parents and nonce.
Push the tx to the fullnode.
Fullnode will send the tx back via websocket since we are subscribed to at least 1 address on it.
Process the tx received via websocket and calculate changes in balances, utxos, etc.

Once the push-tx call successfully returns we can safely assume that it was accepted and propagated to the network, but the wallet is only made aware of this after the fullnode sends the tx via websocket. This wait period can vary depending on the network, fullnode and connection RTT, and can take up to a few seconds.

The main goal of this PR is to make the wallet process the transaction without the wait period since we already know its contents and that it is accepted when step 4 ends.
This will save the wallet a few seconds between each tx which will increase the maximum tx throughput of the wallets while not affecting the normal usage.

Acceptance Criteria

Convert Transaction instance to websocket tx format (IHistoryTx interface)
Add the converted tx to storage after it is accepted by the fullnode on the SendTransaction facade.
Process tx as soon as possible.

Security Checklist

Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

codecov · 2025-02-04T16:35:51Z

Codecov Report

Attention: Patch coverage is 66.66667% with 24 lines in your changes missing coverage. Please review.

Project coverage is 65.80%. Comparing base (9c78996) to head (aabb513).

Files with missing lines	Patch %	Lines
src/utils/transaction.ts	68.62%	14 Missing and 2 partials ⚠️
src/new/sendTransaction.ts	0.00%	6 Missing ⚠️
src/models/p2sh.ts	0.00%	1 Missing ⚠️
src/models/script_data.ts	0.00%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (65.80%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (9c78996) and HEAD (aabb513). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (9c78996) HEAD (aabb513)

2 1

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #813       +/-   ##
===========================================
- Coverage   81.02%   65.80%   -15.23%     
===========================================
  Files          91       91               
  Lines        7190     7259       +69     
  Branches     1517     1537       +20     
===========================================
- Hits         5826     4777     -1049     
- Misses       1351     2399     +1048     
- Partials       13       83       +70

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…nd-to-storage * origin/master: tests: check tx processing on metadata updates (#812)

r4mmer · 2025-02-11T13:28:25Z

__tests__/integration/helpers/wallet.helper.ts

-export async function createTokenHelper(hWallet, name, symbol, amount, options) {
+export async function createTokenHelper(hWallet, name, symbol, amount, options = {}) {


unrelated to line
All changes in this file are related to typing, some jsdoc types that were not converted into ts types.
This removes some typing alerts from the tests.
Obs: We don't have alerts on the CI because we exclude tests from tsc

r4mmer · 2025-02-11T13:32:24Z

src/api/txApi.ts

-  getTransaction(id, resolve): Promise<void | AxiosResponse<TransactionSchema>> {
+  getTransaction(id, resolve): Promise<void> {


The getTransaction method does not return a TransactionSchema, it passes the TransactionSchema instance to the resolve argument.
We could type resolve: (a: TransactionSchema) => void which is correct, but some other parts of the lib break due to being reliant on the implicit typing.

r4mmer · 2025-02-11T13:38:30Z

src/new/sendTransaction.ts

@@ -422,6 +424,23 @@ export default class SendTransaction extends EventEmitter {
              throw new WalletError(ErrorMessages.TRANSACTION_IS_NULL);
            }
            this.transaction.updateHash();
+            if (this.storage) {


This is the core of the PR.

When we push a transaction we need to convert it from a Transaction instance to a IHistoryTx (which is what is received via websocket) and add it to the storage with normal processing.

If we are creating a token we should also add it to the tokens here because the fullnode may not know that this token exists when we ask for the name and symbol for this UID. It also saves us from making an extraneous API call.

src/utils/storage.ts

src/utils/transaction.ts

…nd-to-storage * origin/master: fix: utxo selection for getUtxosForAmount (#818)

pedroferreira1 · 2025-02-25T13:28:15Z

src/storage/storage.ts

+    await this.addToken(this.getNativeTokenData());
+  }
+
+  async addToken(data: ITokenData): Promise<void> {


Missing docstring

pedroferreira1 · 2025-02-25T13:37:10Z

src/utils/storage.ts

+      const storeToken = await store.getToken(uid);
+      if (storeToken === null) {
+        // saveToken will ignore the meta and save only the token config
+        await store.saveToken(tokenData);
+      }


Isn't this the same as the new storage.addToken method created?

tuliomir · 2025-02-26T14:50:11Z

__tests__/utils/transaction.test.ts

+    tx.hash = 'no-outputs-case';
+    tx.inputs = [new Input('no-outputs', 0)];
+    await expect(transaction.convertTransactionToHistoryTx(tx, storage)).rejects.toThrow(
+      'Index outsite of tx output array bounds'


fix: minor typo

Suggested change

'Index outsite of tx output array bounds'

'Index outside of tx output array bounds'

…nd-to-storage * origin/master: tests: use rocksdb in the integration tests (#819) feat: transaction template (#808)

feat: process tx after push

94e96e8

r4mmer self-assigned this Feb 4, 2025

r4mmer requested a review from pedroferreira1 as a code owner February 4, 2025 16:33

r4mmer added 11 commits February 5, 2025 18:00

chore: add comments explaining steps

8b978c4

chore: linter changes

8d9b598

feat: add token being created on storage

b55524f

feat: create token txs should use the uid as output.token

7882a17

Merge remote-tracking branch 'origin/master' into feat/push-tx-and-se…

457dc12

…nd-to-storage * origin/master: tests: check tx processing on metadata updates (#812)

feat: fetch tx from api when it is not in storage

32f21f6

chore: linter changes

cfe92eb

chore: remove typing on api

d153f63

chore: adding array bounds to error message

6a23eb6

feat: change index test

53b0eb1

feat: check we already have the token first

a72b608

r4mmer commented Feb 11, 2025

View reviewed changes

r4mmer requested review from tuliomir, glevco and pedroferreira1 and removed request for pedroferreira1 and glevco February 11, 2025 13:56

tuliomir reviewed Feb 12, 2025

View reviewed changes

src/utils/storage.ts Show resolved Hide resolved

src/utils/transaction.ts Outdated Show resolved Hide resolved

src/utils/transaction.ts Outdated Show resolved Hide resolved

src/utils/transaction.ts Outdated Show resolved Hide resolved

r4mmer added 4 commits February 21, 2025 13:23

Merge remote-tracking branch 'origin/master' into feat/push-tx-and-se…

f48b51f

…nd-to-storage * origin/master: fix: utxo selection for getUtxosForAmount (#818)

chore: review changes

e893e7c

chore: linter changes

9abd019

tests: transaction util test

e5cef34

r4mmer requested a review from tuliomir February 24, 2025 16:01

Merge branch 'master' into feat/push-tx-and-send-to-storage

3371335

pedroferreira1 reviewed Feb 25, 2025

View reviewed changes

tuliomir previously approved these changes Feb 26, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into feat/push-tx-and-se…

d8f50a0

…nd-to-storage * origin/master: tests: use rocksdb in the integration tests (#819) feat: transaction template (#808)

r4mmer dismissed tuliomir’s stale review via d8f50a0 February 27, 2025 18:48

chore: fix type name on import

aabb513

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: process tx after push #813

feat: process tx after push #813

r4mmer commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading

r4mmer Feb 11, 2025

r4mmer Feb 11, 2025

r4mmer Feb 11, 2025

pedroferreira1 Feb 25, 2025

pedroferreira1 Feb 25, 2025

tuliomir Feb 26, 2025

		export async function createTokenHelper(hWallet, name, symbol, amount, options) {
		export async function createTokenHelper(hWallet, name, symbol, amount, options = {}) {

		getTransaction(id, resolve): Promise<void \| AxiosResponse<TransactionSchema>> {
		getTransaction(id, resolve): Promise<void> {

	'Index outsite of tx output array bounds'
	'Index outside of tx output array bounds'

feat: process tx after push #813

Are you sure you want to change the base?

feat: process tx after push #813

Conversation

r4mmer commented Feb 4, 2025 • edited Loading

Motivation

Acceptance Criteria

Security Checklist

codecov bot commented Feb 4, 2025 • edited Loading

Codecov Report

r4mmer Feb 11, 2025

Choose a reason for hiding this comment

r4mmer Feb 11, 2025

Choose a reason for hiding this comment

r4mmer Feb 11, 2025

Choose a reason for hiding this comment

pedroferreira1 Feb 25, 2025

Choose a reason for hiding this comment

pedroferreira1 Feb 25, 2025

Choose a reason for hiding this comment

tuliomir Feb 26, 2025

Choose a reason for hiding this comment

r4mmer commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading