refactor: create TransactionService class #3646

konstantinabl · 2025-04-07T18:12:13Z

Description:

This PR refactors the JSON-RPC relay by extracting transaction-related functionality from the eth.ts file into a dedicated TransactionService class and moving shared code into a CommonService. This improves code organization, testability, and maintainability.

Changes:

Created new TransactionService to handle all transaction-related RPC methods
Moved methods used only from the transaction related methods to the TransactionService - sendRawTransactionProcessor, prune0x, parseRawTxAndPrecheck, sendRawTransactionErrorHandler
Moved common static variables into a CommonService class
Changed the way we use ethExecutionsCounter - moved it to the metric service as an event listener, similar to other metrics we have
Created TransactionReceiptFactory to standardize creation of transaction receipts
Added JSDoc where missing

Moved methods to TransactionService

getTransactionByBlockHashAndIndex
getTransactionByBlockNumberAndIndex
getTransactionByHash
getTransactionReceipt
sendRawTransaction
sendTransaction
signTransaction
sign

Fixes:

#3570
#3622

quiet-node

Great work! Much cleaner and more organized now. Left some Qs and suggestions around.

packages/relay/src/lib/services/factories/ITransactionReceiptFactoryParams.ts

packages/relay/src/lib/services/transactionService/TransactionService.ts

quiet-node · 2025-04-09T17:19:39Z

packages/relay/src/lib/services/transactionService/TransactionService.ts

+   * @param requestDetails The request details for logging and tracking
+   * @returns {Promise<string>} A promise that resolves to the gas price as a hex string
+   */
+  private async getCurrentGasPriceForBlock(blockHash: string, requestDetails: RequestDetails): Promise<string> {


Hmmm I think we should think about migrating these private methods, e.g. getCurrentGasPriceForBlock(), getCurrentNetworkExchangeRateInCents(), etc., to a different module as to me these are not necessarily related to transaction per se but more of helper or utility function. wdyt?

the thing is they are only used here, thus i moved them i nthe service, since only this service need them. Maybe we can create a util file for each service, but that kinda seems like an overkill

Ah, I see thanks for clarifying. Yeah, maybe if we consider grouping services like BlockService, AccountService, TransactionService, FeeService, etc. into the ethService folder like discussed last time we can migrate these into an ethCommonService or perhaps create a new module like ethServiceHelper or ethServiceUtils to house all the helper functions. That way, we avoid cluttering the space with public transaction-related or account-related etc. methods. Just a suggestion though it’s not within the scope of this PR.

packages/relay/src/lib/services/index.ts

packages/relay/src/lib/eth.ts

quiet-node · 2025-04-09T17:47:17Z

packages/relay/src/lib/eth.ts

+      this.eventEmitter.emit(constants.EVENTS.ETH_EXECUTION, {
+        method: EthImpl.ethEstimateGas,
+        functionSelector: callData!.substring(0, constants.FUNCTION_SELECTOR_CHAR_LENGTH),
+        from: transaction.from || '',
+        to: transaction.to || '',
+        requestDetails: requestDetails,
+      });
    }


Nice, I like this centralized solution, but it does make me a bit angsty. Let’s ensure the metrics work as expected and use the correct keyword for the metric names. Otherwise, Grafana might fail to collect them.

Not sure what exactly do you think should be changed?

Oh it's not about changing things but just make sure that we don't accidentally rename the metrics name or labels or arguments. Make sure we preserve them so no issue on Grafana. Please resolve the conversation if you're confident we got things right.

packages/relay/src/logsBloomUtils.ts

packages/relay/src/lib/services/transactionService/TransactionService.ts

packages/relay/src/lib/eth.ts

packages/relay/src/lib/services/transactionService/TransactionService.ts

…ion methods from eth.ts to TransactionService Signed-off-by: Konstantina Blazhukova <[email protected]>

Signed-off-by: Konstantina Blazhukova <[email protected]>

packages/relay/src/lib/services/transactionService/TransactionService.ts

quiet-node · 2025-04-11T15:19:48Z

packages/relay/src/lib/services/transactionService/TransactionService.ts

+  /**
+   * Removes the '0x' prefix from a string if present
+   * @param input The input string
+   * @returns {string} The input string without the '0x' prefix
+   */
+  private prune0x(input: string): string {
+    return input.startsWith(CommonService.EMPTY_HEX) ? input.substring(2) : input;
+  }
+


Quick note I see an exact prune0x in the SDKClient class too. If it's newly introduced then I guess we can refactor the other one in SDKClient to utils and reuse it but if it's migrated then nvm we can roll with it since it's outside the scope. Anywho if changes needed we can do it in follow-up PRs.

This would require using common service in sdk client and do more changes, can we address in another PR?

Gotcha — yeah, it was just a quick observation. We should do it in another PR if it requires many changes. Also I see several topics that need to be addressed in different PRs particularly for this topic. Let’s create a centralized technical debt issue for the ETH refactor and house all those topics there. We can create the actual tickets when we actually work on them so we have clear priorities and don’t clutter the ticket board too much.

quiet-node

Great work! Just left some notes and suggestions if you agree let's create tickets to keep track! Otherwise all good on my end

Signed-off-by: Konstantina Blazhukova <[email protected]>

Signed-off-by: Konstantina Blazhukova <[email protected]> Signed-off-by: Logan Nguyen <[email protected]>

konstantinabl linked an issue Apr 7, 2025 that may be closed by this pull request

Refactor: Create TransactionService class #3570

Closed

konstantinabl changed the base branch from main to poc-eth-service-apporach April 7, 2025 21:21

konstantinabl self-assigned this Apr 8, 2025

konstantinabl force-pushed the 3570-refactor-create-transactionservice-class branch from dc202fb to 58d2386 Compare April 8, 2025 11:32

konstantinabl changed the base branch from poc-eth-service-apporach to main April 8, 2025 11:32

konstantinabl added this to the 0.69.0 milestone Apr 8, 2025

konstantinabl changed the base branch from main to poc-eth-service-apporach April 8, 2025 13:01

konstantinabl changed the title ~~3570 refactor create transactionservice class~~ refactor: create TransactionService class Apr 8, 2025

konstantinabl marked this pull request as ready for review April 8, 2025 13:14

konstantinabl requested review from a team as code owners April 8, 2025 13:14

konstantinabl requested a review from Ferparishuertas April 8, 2025 13:14

konstantinabl force-pushed the 3570-refactor-create-transactionservice-class branch from 081948b to b790ba2 Compare April 8, 2025 13:49

konstantinabl added the enhancement New feature or request label Apr 8, 2025

quiet-node reviewed Apr 9, 2025

View reviewed changes

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Outdated Show resolved Hide resolved

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Outdated Show resolved Hide resolved

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Outdated Show resolved Hide resolved

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/eth.ts Outdated Show resolved Hide resolved

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Show resolved Hide resolved

simzzz reviewed Apr 10, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Show resolved Hide resolved

quiet-node modified the milestones: 0.69.0, 0.68.0 Apr 10, 2025

konstantinabl added 7 commits April 11, 2025 12:32

Creates transactionService, TransactionReceiptFactory, moves transact…

f6432b5

…ion methods from eth.ts to TransactionService Signed-off-by: Konstantina Blazhukova <[email protected]>

removes unecessary imports

924b0c8

Signed-off-by: Konstantina Blazhukova <[email protected]>

splits sendRawTransactionProcessor into chunks

1f600ab

Signed-off-by: Konstantina Blazhukova <[email protected]>

fixes unit tests

68286fb

Signed-off-by: Konstantina Blazhukova <[email protected]>

orders methods alphabetically and adds JSDoc

4fd3079

Signed-off-by: Konstantina Blazhukova <[email protected]>

improves transaction receipt factory

508f3ef

Signed-off-by: Konstantina Blazhukova <[email protected]>

fixes return type of method in TransactionService

f7a2eef

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl added 3 commits April 11, 2025 12:32

removes unused methods

049edb2

Signed-off-by: Konstantina Blazhukova <[email protected]>

removes unused imports

ca39963

Signed-off-by: Konstantina Blazhukova <[email protected]>

fixes imports

fdc0b6b

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl force-pushed the 3570-refactor-create-transactionservice-class branch from bc8fa3b to fdc0b6b Compare April 11, 2025 09:34

konstantinabl added 8 commits April 11, 2025 12:38

simplify services index file

a720b41

Signed-off-by: Konstantina Blazhukova <[email protected]>

removes unused vars from eth.ts and order properties alphabetically

16b1467

Signed-off-by: Konstantina Blazhukova <[email protected]>

makes EMPTY_BLOOM a centralized constant

efa8f83

Signed-off-by: Konstantina Blazhukova <[email protected]>

adresses nits in transaction service

f21fe1f

Signed-off-by: Konstantina Blazhukova <[email protected]>

removes unused methods in eth.ts

29a8399

Signed-off-by: Konstantina Blazhukova <[email protected]>

fixes unit tests

952148c

Signed-off-by: Konstantina Blazhukova <[email protected]>

simplifies getTransactionReceipt

5f26377

Signed-off-by: Konstantina Blazhukova <[email protected]>

moves transaction receipt params interfaces to factory calss

294fa3b

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl requested review from simzzz and quiet-node April 11, 2025 13:53

simzzz approved these changes Apr 11, 2025

View reviewed changes

acuarica reviewed Apr 11, 2025

View reviewed changes

packages/relay/src/lib/services/transactionService/TransactionService.ts Show resolved Hide resolved

packages/relay/src/lib/services/transactionService/TransactionService.ts Show resolved Hide resolved

quiet-node reviewed Apr 11, 2025

View reviewed changes

quiet-node approved these changes Apr 11, 2025

View reviewed changes

konstantinabl requested a review from acuarica April 17, 2025 09:55

konstantinabl merged commit 073d034 into poc-eth-service-apporach Apr 17, 2025
6 checks passed

konstantinabl deleted the 3570-refactor-create-transactionservice-class branch April 17, 2025 10:08

This was referenced Apr 17, 2025

refactor: Rename interfaces in relay repo #3705

Open

refactor: add event types #3706

Open

refactor: create new eventEmitter instances in classes instead of passing the same instance #3707

Open

konstantinabl added a commit that referenced this pull request Apr 25, 2025

refactor: create TransactionService class (#3646)

cb5bb24

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl added a commit that referenced this pull request May 5, 2025

refactor: create TransactionService class (#3646)

7b0fc8e

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl added a commit that referenced this pull request May 7, 2025

refactor: create TransactionService class (#3646)

d9f8649

Signed-off-by: Konstantina Blazhukova <[email protected]>

konstantinabl added a commit that referenced this pull request May 7, 2025

refactor: create TransactionService class (#3646)

88ddc66

Signed-off-by: Konstantina Blazhukova <[email protected]>

quiet-node pushed a commit that referenced this pull request May 7, 2025

refactor: create TransactionService class (#3646)

dbfaf24

Signed-off-by: Konstantina Blazhukova <[email protected]>

quiet-node pushed a commit that referenced this pull request May 7, 2025

refactor: create TransactionService class (#3646)

e39ba37

Signed-off-by: Konstantina Blazhukova <[email protected]> Signed-off-by: Logan Nguyen <[email protected]>

refactor: create TransactionService class #3646

refactor: create TransactionService class #3646

Uh oh!

Conversation

konstantinabl commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quiet-node left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

quiet-node Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

konstantinabl Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

quiet-node Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

quiet-node Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

konstantinabl Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

quiet-node Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

quiet-node Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

konstantinabl Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

quiet-node Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

quiet-node left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

konstantinabl commented Apr 7, 2025 •

edited

Loading

quiet-node Apr 11, 2025 •

edited

Loading

quiet-node Apr 11, 2025 •

edited

Loading