REST API: Add Header-Only parameter to blocks endpoint, return error if there are too many transactions. #1241
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1241 +/- ##
===========================================
+ Coverage 60.92% 61.11% +0.19%
===========================================
Files 52 52
Lines 8488 8512 +24
===========================================
+ Hits 5171 5202 +31
+ Misses 2859 2848 -11
- Partials 458 462 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
idb/postgres/postgres.go
Outdated
| @@ -480,6 +481,9 @@ func (db *IndexerDb) GetBlock(ctx context.Context, round uint64, options idb.Get | |||
| results := make([]idb.TxnRow, 0) | |||
| for txrow := range out { | |||
| results = append(results, txrow) | |||
| if uint64(len(results)) > options.MaxTransactionsLimit { | |||
There was a problem hiding this comment.
Please add a limit to the idb.TransactionFilter parameters above to avoid fetching all transactions.
Be careful about off-by-one problems, I think the limit would need to be options.MaxTransactionsLimit + 1 in order for this check to be detected.
I'm not 100% certain, but I think there is also a potential deadlock related to the out channel. db.yieldTxnsThreadSimple would be stuck filling the out channel if we return early, so you need to make sure the range function exits.
There was a problem hiding this comment.
I think with the limit filter added, we're ok with out channel. When line 484 is true, out would be empty.
There was a problem hiding this comment.
I think you're right, but the return inside the range seems dangerous. Here's an example of clearing out the channel (I thought there were others but this is the only one I could find): https://github.com/algorand/indexer/blob/develop/api/handlers.go#L1216
Could you move the if statement to the outside of the range statement? It should still be efficient, thanks to the limit, but would also ensure that the channel is closed.
winder
changed the title
Summary
#332 Adding header-only parameter to GET /v2/blocks/{round-number} which would return a block excluding the transactions. If the number of the transactions in a request block exceeds MaxTransactionsLimit, the endpoint return err code 400 when header-only parameter isn't set.
Test Plan
new unit tests and integration tests.