Cleanup record access #2792
Cleanup record access #2792
Conversation
…ords this is pointless) Signed-off-by: tinker-michaelj <[email protected]>
…unt=180 Signed-off-by: tinker-michaelj <[email protected]>
Signed-off-by: tinker-michaelj <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2792 +/- ##
=========================================
Coverage 95.76% 95.77%
- Complexity 9656 9667 +11
=========================================
Files 699 699
Lines 26377 26395 +18
Branches 2710 2714 +4
=========================================
+ Hits 25260 25279 +19
+ Misses 581 580 -1
Partials 536 536
Continue to review full report at Codecov.
|
tinker-michaelj
requested review from
rbair23,
nathanklick and
Neeharika-Sompalli
|
I'm taking a deeper look at the PR. If we are going to add a mechanism to limit the number of returned records (and we really do need this), then we should also talk about pagination or watermarking. Since the number of transactions change all the time, I don't think we can do pagination (it would be basically worthless). Instead we should allow the user to say "get me the next N number of records newer than time T" or "newer than record R", or something along those lines. And we have a cap (as proposed in this PR) for the number of records returned. We can do that as a second PR, but we must recognize that this PR does introduce a breaking change in our API behavior and we need to make sure we document it as such as well. |
Created a PR in hedera-protobufs to support this watermarking. |
hedera-node/src/main/java/com/hedera/services/queries/answering/AnswerFunctions.java
Show resolved
Hide resolved
| @@ -112,6 +153,6 @@ public HederaFunctionality canonicalFunction() { | |||
| @Override | |||
| public Optional<SignedTxnAccessor> extractPaymentFrom(final Query query) { | |||
| final var paymentTxn = query.getCryptoGetAccountRecords().getHeader().getPayment(); | |||
| return Optional.ofNullable(SignedTxnAccessor.uncheckedFrom(paymentTxn)); | |||
| return Optional.of(SignedTxnAccessor.uncheckedFrom(paymentTxn)); | |||
There was a problem hiding this comment.
What was the significance of this change?
There was a problem hiding this comment.
The gRPC accessors never return null, so IntelliJ was marking this Optional.ofNullable() as a code smell.
Description:
GetTxnRecordAnswerthat looks for the theTransactionRecordin theTransactionID's payer account when it is missing from thetxnHistoriesmap.txnHistoriesmap. (Both live for 180 seconds, and are removed simultaneously during record expiration.)CryptoGetAccountRecordsQueryby the global/dynamic propertyledger.records.maxQueryableByAccount=180.Map<String, Object> queryCtxset by theStakedAnswerFlow.