Enhancement: add max int64 checks #1166

shiqizng · 2022-08-05T14:54:42Z

Summary

This PR adds max int64 checks to round, application ID and asset ID in API requests.

Test Plan

Add unit tests for the handler and filter methods.

codecov · 2022-08-05T15:26:09Z

Codecov Report

Merging #1166 (5a7f582) into develop (6b61b08) will increase coverage by 0.42%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1166      +/-   ##
===========================================
+ Coverage    59.85%   60.28%   +0.42%     
===========================================
  Files           51       51              
  Lines         8288     8359      +71     
===========================================
+ Hits          4961     5039      +78     
+ Misses        2868     2862       -6     
+ Partials       459      458       -1

Impacted Files	Coverage Δ
api/error_messages.go	`100.00% <ø> (ø)`
api/handlers.go	`74.73% <100.00%> (+2.05%)`	⬆️
idb/postgres/postgres.go	`64.40% <100.00%> (+0.48%)`	⬆️
fetcher/fetcher.go	`50.87% <0.00%> (-1.32%)`	⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

api/handlers.go

Eric-Warehime · 2022-08-08T17:27:49Z

api/converter_utils.go

@@ -588,6 +589,10 @@ func (si *ServerImplementation) transactionParamsToTransactionFilter(params gene
 	filter.ApplicationID = uintOrDefault(params.ApplicationId)
 	filter.Limit = min(uintOrDefaultValue(params.Limit, si.opts.DefaultTransactionsLimit), si.opts.MaxTransactionsLimit)
 	filter.Round = params.Round
+	// Integer > math.Int64
+	if filter.AssetID > math.MaxInt64 || filter.ApplicationID > math.MaxInt64 || (filter.Round != nil && uint64(*filter.Round) > math.MaxInt64) {


I see that there are a lot more fields in a filter which are uint64 https://github.com/algorand/indexer/blob/develop/idb/idb.go#L196

Why are these the only ones we need to do checks for as opposed to checking all of the supplied filters?

I moved these checks to validateTransactionFilter and added checks for all uint64 fields except for AssetAmount which is type numeric(20) in the DB table.

winder · 2022-08-09T10:46:19Z

idb/postgres/postgres.go

@@ -582,12 +582,12 @@ func buildTransactionQuery(tf idb.TransactionFilter) (query string, whereArgs []
 		partNumber++
 	}
 	if tf.AssetAmountGT != nil {
-		whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'aamt')::bigint > $%d", partNumber))
+		whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'aamt')::numeric(20) > $%d", partNumber))


nice catch, did this cause a failure before?

it was causing a failure for a test case I just added that has a txn with asset amount maxUint64.

* Bump version to 2.13.0-rc1 * Bump version to 2.13.0 * Documentation for data directory. (#1125) Co-authored-by: algobarb <78746954+algobarb@users.noreply.github.com> * Don't lookup big foreign assets. (#1141) * Revert "Bump version to 2.13.0" This reverts commit 0a8af61. * Bump version to 2.13.0 * Fix import performance test runner. (#1133) * Start on round 1 since round 0 is now computed from the genesis file. * Wait for indexer processor to exit. * Better logging for metric collection errors. * Proper support for data directory. * New test script for future release automation. * Revert "Bump version to 2.13.0" This reverts commit 7915890. * Bump version to 2.13.0 * test fixes: Submodule updates (#1144) * Update go-algorand submodule * Fix test failure due to duplicate txns * Add new ledger interface method * Enhancement: remove import validator utility and obsolete ledger for evaluator (#1146) removing a bunch of code and make the random test pass with the new ledger for evaluator * Docs: Readme update (#1149) * Update README header * Testing: Use tempdir instead of /tmp for e2elive test (#1152) * Format misc/*.py with `black` (#1153) * apply black to e2elive.py as well (#1154) * Enhancement: More information about S3 keys searched for and Dockerfile that uses submodule instead of channel (#1151) * Eric's Dockerfile improvements * Update misc/e2elive.py * Bug-Fix: Implement BlockHdrCached + miscellany (#1162) * Enhancement: add max int64 checks (#1166) * state proofs: Indexer Support for State Proofs (#1002) Adds API support to the Indexer for State Proof Transactions and header fields. Co-authored-by: Will Winder <wwinder.unh@gmail.com> Co-authored-by: Will Winder <wwinder.unh@gmail.com> * Bump version to 2.14.0-rc1 * Stop Panics if no config is supplied (#1180) Give a default config if not supplied to stop panics. * Fix spec name collisions. (#1182) * Update go-algorand submodule to v3.9.1-beta (#1185) * Bump version to 2.14.0-rc2 * Disable deadlock detection (#1186) * Add support for new block header: TxnRoot SHA256 (#989) * Accept yaml and yml configuration files. (#1181) * Fix bug in reveals lookup (#1198) * Fix bug in reveals lookup (#1198) * Bump version to 2.14.0-rc3 * add state proof example with high reveal index - from betanet (#1199) * Devops: Bump go-algorand submodule to v3.9.2-beta (#1203) * Bump version to 2.14.0-rc4 * enhancement: Clarify REST query parameters for accounts search (#1201) * update description for /v2/accounts * cicd: add darwin arm64 support to release script (#1169) * Bump version to 2.14.0 * Downgrade mockery to prevent incorrect deprecation warning. (#1211) * Enhancement: update e2e test policy (#1197) *update e2e test policy * Fix release 2.14.0 (#1214) * Accept yaml and yml configuration files. (#1181) * Fix bug in reveals lookup (#1198) * add state proof example with high reveal index - from betanet (#1199) * enhancement: Clarify REST query parameters for accounts search (#1201) * update description for /v2/accounts * cicd: add darwin arm64 support to release script (#1169) * Downgrade mockery to prevent incorrect deprecation warning. (#1211) * Enhancement: update e2e test policy (#1197) *update e2e test policy * Update test expected value: transaction root sha256 Co-authored-by: AlgoStephenAkiki <85183435+AlgoStephenAkiki@users.noreply.github.com> Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com> Co-authored-by: algoidan <79864820+algoidan@users.noreply.github.com> Co-authored-by: shiqizng <80276844+shiqizng@users.noreply.github.com> Co-authored-by: algolucky <lucky.baar@algorand.com> Co-authored-by: Will Winder <wwinder.unh@gmail.com> Co-authored-by: DevOps Service <devops-service@algorand.com> Co-authored-by: Will Winder <wwinder.unh@gmail.com> Co-authored-by: algobarb <78746954+algobarb@users.noreply.github.com> Co-authored-by: Barbara Poon <barbara.poon@algorand.com> Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com> Co-authored-by: shiqizng <80276844+shiqizng@users.noreply.github.com> Co-authored-by: AlgoStephenAkiki <85183435+AlgoStephenAkiki@users.noreply.github.com> Co-authored-by: John Lee <john.lee@algorand.com> Co-authored-by: Or Aharonee <17099688+Aharonee@users.noreply.github.com> Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com> Co-authored-by: algoidan <79864820+algoidan@users.noreply.github.com> Co-authored-by: algolucky <lucky.baar@algorand.com>

add max int64 checks

c8cb7b3

shiqizng added Enhancement New feature or request Team Lamprey labels Aug 5, 2022

shiqizng self-assigned this Aug 5, 2022

shiqizng mentioned this pull request Aug 5, 2022

Enhancement: update indexer table schema #1143

Closed

fix failing test

fc97c11

shiqizng requested review from winder, AlgoStephenAkiki and Eric-Warehime August 5, 2022 15:28

winder reviewed Aug 8, 2022

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

Eric-Warehime reviewed Aug 8, 2022

View reviewed changes

addressed PR suggestions, added more checks to filter validation

01fffbe

shiqizng requested review from winder and Eric-Warehime August 8, 2022 21:09

winder reviewed Aug 9, 2022

View reviewed changes

winder approved these changes Aug 9, 2022

View reviewed changes

update error message

5a7f582

Eric-Warehime approved these changes Aug 9, 2022

View reviewed changes

winder merged commit 8c49d86 into develop Aug 9, 2022

winder deleted the shiqi/maxintChecks branch August 9, 2022 16:49

This was referenced Aug 10, 2022

FOR REVIEW ONLY: indexer 2.14.0 into master #1170

Merged

FOR REVIEW ONLY: indexer 2.14.0 into develop #1171

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement: add max int64 checks #1166

Enhancement: add max int64 checks #1166

shiqizng commented Aug 5, 2022

codecov bot commented Aug 5, 2022 •

edited

Loading

Eric-Warehime Aug 8, 2022

shiqizng Aug 8, 2022

winder Aug 9, 2022

shiqizng Aug 9, 2022

Enhancement: add max int64 checks #1166

Enhancement: add max int64 checks #1166

Conversation

shiqizng commented Aug 5, 2022

Summary

Test Plan

codecov bot commented Aug 5, 2022 • edited Loading

Codecov Report

Eric-Warehime Aug 8, 2022

Choose a reason for hiding this comment

shiqizng Aug 8, 2022

Choose a reason for hiding this comment

winder Aug 9, 2022

Choose a reason for hiding this comment

shiqizng Aug 9, 2022

Choose a reason for hiding this comment

codecov bot commented Aug 5, 2022 •

edited

Loading