fix: Update getTransactionByBlockHashOrBlockNumAndIndex to pass timestamp to MAPI calls
#3049
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…estamp to MAPI calls Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
victor-yanev
requested review from
AlfredoG87,
ebadiere,
lukelee-sl and
a team
as code owners
ebadiere
reviewed
Oct 2, 2024
There was a problem hiding this comment.
The "Tests" seem to fail consistently:
3 failing
1) HAPI Service
should be able to reset all counter upon reinitialization of the SDK Client:
AssertionError: expected 35999 to equal 36000
+ expected - actual
-35999
+36000
at Context.<anonymous> (tests/lib/hapiService.spec.ts:132:48)
at Generator.next (<anonymous>)
at /home/runner/_work/hedera-json-rpc-relay/hedera-json-rpc-relay/packages/relay/tests/lib/hapiService.spec.ts:27:71
at new Promise (<anonymous>)
at __awaiter (tests/lib/hapiService.spec.ts:23:12)
at Context.<anonymous> (tests/lib/hapiService.spec.ts:124:20)
at processImmediate (node:internal/timers:483:21)
2) Open RPC Specification
should execute "eth_getTransactionByBlockHashAndIndex":
Error invoking RPC: Could not find mock for:
{
"method": "get",
"url": "contracts/results?block.hash=0x3c08bbbee74d287b1dcd3f0ca6d1d2cb92c90883c4acf9747de9f3f3162ad25b999fc7e86699f60f2a3fb3ed9a646c6b×tamp=gte:1651560386.060890949×tamp=lte:1651560389.060890949&transaction.index=77&limit=100&order=asc"
}
3) Open RPC Specification
should execute "eth_getTransactionByBlockNumberAndIndex":
Error invoking RPC: Could not find mock for:
{
"method": "get",
"url": "contracts/results?block.number=3×tamp=gte:1651560386.060890949×tamp=lte:1651560389.060890949&transaction.index=77&limit=100&order=asc"
}
```
Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
@ebadiere Fixed |
Nana-EC
reviewed
Oct 3, 2024
|
|
||
| export function contractResultsByNumberByIndexURL(number: number, index: number): string { | ||
| return `contracts/results?block.number=${number}&transaction.index=${index}&limit=100&order=asc`; | ||
| export function getQueryParams(params: object) { |
There was a problem hiding this comment.
Doesn't this function already exist in helpers.ts?
Could it be reused?
There was a problem hiding this comment.
Yes, it could be reused but it needs to be extended to also support arrays for values in the params object, I will extend it and remove this one. Thanks!
|
|
||
| const hapiServiceInstance = new HAPIService(logger, registry, hbarLimiter, cacheService); | ||
| const hapiServiceInstance = new HAPIService(logger, registry, hbarLimiter, cacheService, eventEmitter); |
There was a problem hiding this comment.
Seems like an unrelated change, is this a bug you realized that you're fixing.
Is so please update the PR description with the details.
There was a problem hiding this comment.
Yes, eth-helpers.ts wasn't updated to pass the new EventEmitter parameter of HAPIService when those changes which modified the constructor were introduced, and I was getting TypeScript errors for this file.
ebadiere
previously approved these changes
Oct 3, 2024
…BlockNumAndIndex' Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
victor-yanev
force-pushed
the
3042-Update-eth_getTransactionByBlockHashOrBlockNumAndIndex
branch
from
9de65e7
to
5a1c6a0
Compare
Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
|
|
After discussing this with xin, it seems like those changes were not needed |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3049 +/- ##
==========================================
- Coverage 84.83% 84.81% -0.02%
==========================================
Files 59 59
Lines 3937 3939 +2
Branches 786 787 +1
==========================================
+ Hits 3340 3341 +1
Misses 357 357
- Partials 240 241 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
eth_getTransactionByBlockHashAndIndexandeth_getTransactionByBlockNumberAndIndexto pass a timestamp range of the block to subsequent calls togetContractResultsRelated issue(s):
Fixes #3042
Notes for reviewer:
Checklist