feat: added an option for retrieving transaction records from either the Mirror Node or the Consensus Node.

[quiet-node]

Description:

• added GET_RECORD_DEFAULT_TO_CONSENSUS_NODE to configuration docs. This configuration allows the Relay to retrieve transaction record by making calls to Consensus Node (CN) or Mirror Node (MN) based on its input. Default to "false" for MN calls.

• added new wrapper class TransactionService with getTransactionStatusAndMetrrics() method. This method is the implementation for the logic mentioned above. If GET_RECORD_DEFAULT_TO_CONSENSUS_NODE is set to "true", calls will be redirected to CN. Otherwise, calls will be redirected to MN. MirrorNodeClient is injected from eth.ts level. Unit tests are included.

• modified getTransferAmountSumForAccount so it can be compatible with transfers array returned from MN.

• added getTransactionStatusAndMetrrics to executeTransaction() and executeTransaction().

• removed unused methods and added JSDoc to methods in SDKClient, sdkClient.executeGetTransactionRecord() and sdkClient.getTransactionMetrics() as they are now replaced by getTransactionStatusAndMetrrics(). Also added JSDoc comments for all methods in SDKClient.

• Modified and added new tests in sdkClient.spec.ts reflect the new logic.

• added a new IMirrorNode.ts type file and host all interfaces and types that are related to Mirror Node.

Related issue(s):

Fixes #2706

Notes for reviewer:

Checklist

• Documented (Code comments, README, etc.)
• Tested (unit, integration, etc.)

[github-actions]

Tests

       3 files     199 suites   40s ⏱️
   996 tests    995 ✔️ 1 💤 0 ❌
1 009 runs  1 008 ✔️ 1 💤 0 ❌

Results for commit 79937d4.

♻️ This comment has been updated with latest results.

[github-actions]

Acceptance Tests

     24 files  300 suites   37m 31s ⏱️
   611 tests 591 ✔️ 4 💤 16 ❌
1 046 runs  998 ✔️ 4 💤 44 ❌

Results for commit 79937d4.

♻️ This comment has been updated with latest results.

[Ivo-Yankov]

Looks really good. I think we should run the Hbar Limiter tests two times - once with GET_RECORD_DEFAULT_TO_CONSENSUS_NODE=true and once with false. Ideally when it is using the Mirror node the total untracked spent amount by the operator should be less. In those tests there is a delta variable, currently set to 0.02. Try decreasing it and see if we are closer to 0 when using the MN as compared to CN

[quiet-node]

Looks really good. I think we should run the Hbar Limiter tests two times - once with GET_RECORD_DEFAULT_TO_CONSENSUS_NODE=true and once with false. Ideally when it is using the Mirror node the total untracked spent amount by the operator should be less. In those tests there is a delta variable, currently set to 0.02. Try decreasing it and see if we are closer to 0 when using the MN as compared to CN

Yes I agree! It is a good point! Pushing updates soon! Thanks!

[quiet-node]

Looks really good. I think we should run the Hbar Limiter tests two times - once with GET_RECORD_DEFAULT_TO_CONSENSUS_NODE=true and once with false. Ideally when it is using the Mirror node the total untracked spent amount by the operator should be less. In those tests there is a delta variable, currently set to 0.02. Try decreasing it and see if we are closer to 0 when using the MN as compared to CN

@Ivo-Yankov updated new changes to run hbarLimiter with another round with calls being made to mirror node. Delta just went down a little not too much though but still the new solution did its job. Please take a look when you have a chance!

[Ivo-Yankov]

Unblocking the requested changes.

[victor-yanev]

Great job on the improvements; the code is certainly cleaner than before. However, passing the TransactionService around within methods is still creating unnecessary coupling between classes, which we should address.

To avoid this and simplify the dependency structure, we can take the following steps:

1. Follow the dependency tree: EthImpl -> HapiService -> AccountService, TransactionService, ContractService, FeeService -> SdkClient, MirrorNodeClient. We can consider opening separate issues to extract logic for accounts, contracts, and fees into distinct services.

2. Keep the logic for initializing SdkClient into HapiService. This way, HapiService will manage the SdkClient and be the main point of interaction.

3. Remove the direct dependency on TransactionService from EthImpl. Instead, initialize TransactionService inside HapiService, and have HapiService pass the initialized SdkClient to it.

4. Add methods such as createFile, deleteFile, appendFile, etc., directly in HapiService. This should be our main entry point for operations (e.g., instead of calling hapiService.getSdkClient().createFile(), we should directly call hapiService.createFile()).

5. The wrapper logic for executeQuery and executeTransaction should also reside within HapiService. HapiService can then use these wrappers and delegate the actual execution to the SdkClient as needed.

6. With HapiService calling TransactionService to fetch and extract transaction details, it remains agnostic to whether SdkClient or MirrorNodeClient is being used.

This approach untangles the dependencies between classes, resulting in a clear and linear dependency tree.

[quiet-node]

Great job on the improvements; the code is certainly cleaner than before. However, passing the TransactionService around within methods is still creating unnecessary coupling between classes, which we should address.

To avoid this and simplify the dependency structure, we can take the following steps:

1. Follow the dependency tree: EthImpl -> HapiService -> AccountService, TransactionService, ContractService, FeeService -> SdkClient, MirrorNodeClient. We can consider opening separate issues to extract logic for accounts, contracts, and fees into distinct services.
2. Keep the logic for initializing SdkClient into HapiService. This way, HapiService will manage the SdkClient and be the main point of interaction.
3. Remove the direct dependency on TransactionService from EthImpl. Instead, initialize TransactionService inside HapiService, and have HapiService pass the initialized SdkClient to it.
4. Add methods such as createFile, deleteFile, appendFile, etc., directly in HapiService. This should be our main entry point for operations (e.g., instead of calling hapiService.getSdkClient().createFile(), we should directly call hapiService.createFile()).
5. The wrapper logic for executeQuery and executeTransaction should also reside within HapiService. HapiService can then use these wrappers and delegate the actual execution to the SdkClient as needed.
6. With HapiService calling TransactionService to fetch and extract transaction details, it remains agnostic to whether SdkClient or MirrorNodeClient is being used.

This approach untangles the dependencies between classes, resulting in a clear and linear dependency tree.

@victor-yanev absolute amazing points! However, the PR is getting bigger and it would be a little out of scope for this particular ticket, given that couple other tickets are being blocked by this PR. Could you kindly open a new ticket and drop all these amazing suggestions there along with more ideas of reorganizing the codebase?

[sonarcloud]

Quality Gate failed

Failed conditions
5.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

[codecov]

Codecov Report

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

Project coverage is 83.29%. Comparing base (4e485ac) to head (79937d4).
Report is 2 commits behind head on main.

Files	Patch %	Lines
packages/relay/src/lib/clients/sdkClient.ts	73.46%	8 Missing and 5 partials ⚠️
.../services/transactionService/transactionService.ts	86.84%	3 Missing and 2 partials ⚠️
packages/relay/src/formatters.ts	63.63%	3 Missing and 1 partial ⚠️
packages/relay/src/lib/clients/mirrorNodeClient.ts	83.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2782      +/-   ##
==========================================
+ Coverage   77.77%   83.29%   +5.51%     
==========================================
  Files          43       48       +5     
  Lines        3312     3513     +201     
  Branches      702      738      +36     
==========================================
+ Hits         2576     2926     +350     
+ Misses        492      346     -146     
+ Partials      244      241       -3     

Flag	Coverage Δ
relay	80.57% <77.23%> (+1.72%)	⬆️
server	81.69% <ø> (+7.07%)	⬆️
ws-server	97.87% <ø> (+36.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
packages/relay/src/index.ts	100.00% <ø> (ø)
packages/relay/src/lib/eth.ts	84.73% <100.00%> (+3.30%)	⬆️
packages/relay/src/lib/types/IMirrorNode.ts	100.00% <100.00%> (ø)
packages/relay/src/lib/clients/mirrorNodeClient.ts	88.78% <83.33%> (+0.34%)	⬆️
packages/relay/src/formatters.ts	61.70% <63.63%> (ø)
.../services/transactionService/transactionService.ts	86.84% <86.84%> (ø)
packages/relay/src/lib/clients/sdkClient.ts	63.67% <73.46%> (+12.93%)	⬆️

... and 23 files with indirect coverage changes