[Access] Add implementation for usage of the local transaction result in Access API #5306
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5306 +/- ##
==========================================
+ Coverage 56.06% 57.42% +1.35%
==========================================
Files 1026 798 -228
Lines 100116 80463 -19653
==========================================
- Hits 56129 46205 -9924
+ Misses 39688 30565 -9123
+ Partials 4299 3693 -606
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
…eroua/4753-local-index-tx-result
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
There was a problem hiding this comment.
Great job, very big lift. Most of my comments are stylistic and around documentation.
The only thing I am unsure is Initialize, I would prefer to avoid such patterns but if they are absolutely needed then they need to be carefully explained. Additionally I don't think case of "not initialized" should be threaten as sentinel error to me it seems that we require initialization before we can serve clients, in other words our invariant is: "Initialize MUST to be called before serving clients" if this invariant is broken then we should bail out with exception, not a sentinel error.
| // get the latest finalized block from the state | ||
| finalized, err := t.state.Final().Head() | ||
| if err != nil { | ||
| return flow.TransactionStatusUnknown, irrecoverable.NewExceptionf("failed to lookup final header: %w", err) |
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
| return reporter.HighestIndexedHeight() | ||
| } | ||
|
|
||
| func (t *TransactionResultsIndex) checkDataAvailability(height uint64) error { |
There was a problem hiding this comment.
We need some godoc for this function, don't forget to describe errors
| func (suite *Suite) checkTransactionResultResponse( | ||
| err error, | ||
| response *acc.TransactionResult, | ||
| block flow.Block, | ||
| txId flow.Identifier, | ||
| txFailed bool, | ||
| eventsForTx []flow.Event, |
There was a problem hiding this comment.
| func (suite *Suite) checkTransactionResultResponse( | |
| err error, | |
| response *acc.TransactionResult, | |
| block flow.Block, | |
| txId flow.Identifier, | |
| txFailed bool, | |
| eventsForTx []flow.Event, | |
| func (suite *Suite) assertTransactionResultResponse( | |
| err error, | |
| response *acc.TransactionResult, | |
| expectedBlock flow.Block, | |
| expectedTxId flow.Identifier, | |
| expectedTxFailed bool, | |
| expectedEvents []flow.Event, |
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
…dex-tx-result [Access] Add implementation for usage of the local transaction result in Access API
#4753
Add implementation for usage of the local transaction result index for existing Access API endpoints(
GetTransactionResult,GetTransactionResultByIndex,GetTransactionResultsByBlockID) to get transaction results for indexed blocks.The errors for transaction results are requested separately and were implemented as part of the issue #4754.