Skip to content

Commit

Permalink
Fix: Ensure requests for asset/application id 0 check for the correct…
Browse files Browse the repository at this point in the history 
… result (#1622)

* Make assetId and applicationId arguments pointers across several indexer queries for assets, apps, and transactions. Meant to resolve issue 1087, where requesting the 0 indexed asset/application resulted in a search query returning an incorrect creatable.

* Add e2e regression test

---------

Co-authored-by: Jason Paulos <[email protected]>
  • Loading branch information
@gmalouf @jasonpaulos
gmalouf and jasonpaulos authored Aug 9, 2024
1 parent 674516d commit 316221e
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 70 deletions.
18 changes: 9 additions & 9 deletions api/converter_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,17 +645,17 @@ func (si *ServerImplementation) assetParamsToAssetQuery(params generated.SearchF
return idb.AssetsQuery{}, errors.New(errUnableToParseAddress)
}

var assetGreaterThan uint64 = 0
var assetGreaterThan *uint64
if params.Next != nil {
agt, err := strconv.ParseUint(*params.Next, 10, 64)
if err != nil {
return idb.AssetsQuery{}, fmt.Errorf("%s: %v", errUnableToParseNext, err)
}
assetGreaterThan = agt
assetGreaterThan = &agt
}

query := idb.AssetsQuery{
AssetID: uintOrDefault(params.AssetId),
AssetID: params.AssetId,
AssetIDGreaterThan: assetGreaterThan,
Creator: creator,
Name: strOrDefault(params.Name),
Expand All @@ -674,17 +674,17 @@ func (si *ServerImplementation) appParamsToApplicationQuery(params generated.Sea
return idb.ApplicationQuery{}, errors.New(errUnableToParseAddress)
}

var appGreaterThan uint64 = 0
var appGreaterThan *uint64
if params.Next != nil {
agt, err := strconv.ParseUint(*params.Next, 10, 64)
if err != nil {
return idb.ApplicationQuery{}, fmt.Errorf("%s: %v", errUnableToParseNext, err)
}
appGreaterThan = agt
appGreaterThan = &agt
}

return idb.ApplicationQuery{
ApplicationID: uintOrDefault(params.ApplicationId),
ApplicationID: params.ApplicationId,
ApplicationIDGreaterThan: appGreaterThan,
Address: addr,
IncludeDeleted: boolOrDefault(params.IncludeAll),
Expand All @@ -698,8 +698,8 @@ func (si *ServerImplementation) transactionParamsToTransactionFilter(params gene
// Integer
filter.MaxRound = uintOrDefault(params.MaxRound)
filter.MinRound = uintOrDefault(params.MinRound)
filter.AssetID = uintOrDefault(params.AssetId)
filter.ApplicationID = uintOrDefault(params.ApplicationId)
filter.AssetID = params.AssetId
filter.ApplicationID = params.ApplicationId
filter.Limit = min(uintOrDefaultValue(params.Limit, si.opts.DefaultTransactionsLimit), si.opts.MaxTransactionsLimit)
filter.Round = params.Round

Expand Down Expand Up @@ -730,7 +730,7 @@ func (si *ServerImplementation) transactionParamsToTransactionFilter(params gene
filter.RekeyTo = params.RekeyTo

// filter Algos or Asset but not both.
if filter.AssetID != 0 || filter.TypeEnum == idb.TypeEnumAssetTransfer {
if filter.AssetID != nil || filter.TypeEnum == idb.TypeEnumAssetTransfer {
filter.AssetAmountLT = params.CurrencyLessThan
filter.AssetAmountGT = params.CurrencyGreaterThan
} else {
Expand Down
13 changes: 7 additions & 6 deletions api/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func validateTransactionFilter(filter *idb.TransactionFilter) error {
// Round or application-id or asset-id is greater than math.MaxInt64
if filter.MinRound > math.MaxInt64 || filter.MaxRound > math.MaxInt64 ||
(filter.Round != nil && *filter.Round > math.MaxInt64) ||
filter.AssetID > math.MaxInt64 || filter.ApplicationID > math.MaxInt64 {
(filter.AssetID != nil && *filter.AssetID > math.MaxInt64) ||
(filter.ApplicationID != nil && *filter.ApplicationID > math.MaxInt64) {
errorArr = append(errorArr, errValueExceedingInt64)
}

Expand Down Expand Up @@ -304,18 +305,18 @@ func (si *ServerImplementation) LookupAccountAssets(ctx echo.Context, accountID
return badRequest(ctx, errors[0])
}

var assetGreaterThan uint64 = 0
var assetGreaterThan *uint64
if params.Next != nil {
agt, err := strconv.ParseUint(*params.Next, 10, 64)
if err != nil {
return badRequest(ctx, fmt.Sprintf("%s: %v", errUnableToParseNext, err))
}
assetGreaterThan = agt
assetGreaterThan = &agt
}

query := idb.AssetBalanceQuery{
Address: addr,
AssetID: uintOrDefault(params.AssetId),
AssetID: params.AssetId,
AssetIDGT: assetGreaterThan,
IncludeDeleted: boolOrDefault(params.IncludeAll),
Limit: min(uintOrDefaultValue(params.Limit, si.opts.DefaultBalancesLimit), si.opts.MaxBalancesLimit),
Expand Down Expand Up @@ -545,7 +546,7 @@ func (si *ServerImplementation) LookupApplicationByID(ctx echo.Context, applicat
return notFound(ctx, errValueExceedingInt64)
}
q := idb.ApplicationQuery{
ApplicationID: applicationID,
ApplicationID: uint64Ptr(applicationID),
IncludeDeleted: boolOrDefault(params.IncludeAll),
Limit: 1,
}
Expand Down Expand Up @@ -819,7 +820,7 @@ func (si *ServerImplementation) LookupAssetBalances(ctx echo.Context, assetID ui
}

query := idb.AssetBalanceQuery{
AssetID: assetID,
AssetID: &assetID,
AmountGT: params.CurrencyGreaterThan,
AmountLT: params.CurrencyLessThan,
IncludeDeleted: boolOrDefault(params.IncludeAll),
Expand Down
30 changes: 26 additions & 4 deletions api/handlers_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,28 @@ func TestApplicationHandlers(t *testing.T) {
checkAppLocalState(t, &response.AppsLocalStates[0])
})
}

t.Run("app-0-query", func(t *testing.T) {
//////////
// When // We query an app that does not exist
//////////

c, api, rec := setupReq("/v2/applications/:appidx", "appidx", "0")
params := generated.LookupApplicationByIDParams{}
err = api.LookupApplicationByID(c, 0, params)
require.NoError(t, err)

//////////
// Then // The response is 404
//////////

require.Equal(t, http.StatusNotFound, rec.Code, fmt.Sprintf("unexpected return code, body: %s", rec.Body.String()))
data := rec.Body.Bytes()
var response generated.ErrorResponse
err = json.Decode(data, &response)
require.NoError(t, err)
require.Equal(t, "no application found for application-id: 0", response.Message)
})
}

func TestAccountExcludeParameters(t *testing.T) {
Expand Down Expand Up @@ -1930,7 +1952,7 @@ func runBoxCreateMutateDelete(t *testing.T, comparator boxTestComparator) {
err = db.AddBlock(&vb1)
require.NoError(t, err)

opts := idb.ApplicationQuery{ApplicationID: uint64(appid)}
opts := idb.ApplicationQuery{ApplicationID: uint64Ptr(uint64(appid))}

rowsCh, round := db.Applications(context.Background(), opts)
require.Equal(t, uint64(currentRound), round)
Expand Down Expand Up @@ -2524,12 +2546,12 @@ func TestAccounts(t *testing.T) {
require.NoError(t, err)
stxn, _, err := util.DecodeSignedTxn(vb.Block.BlockHeader, vb.Block.Payset[0])
require.NoError(t, err)
assetID := crypto.TransactionIDString(stxn.Txn)
transactionID := crypto.TransactionIDString(stxn.Txn)

///////////
// When // Look up transaction containing this asset
///////////
path = fmt.Sprintf("/v2/transactions/%s", assetID)
path = fmt.Sprintf("/v2/transactions/%s", transactionID)
resp, data = makeRequest(t, listenAddr, path, false)
require.Equal(t, http.StatusOK, resp.StatusCode, fmt.Sprintf("unexpected return code, body: %s", string(data)))
var txn generated.TransactionResponse
Expand All @@ -2552,7 +2574,7 @@ func TestAccounts(t *testing.T) {
///////////
// When // Look up the asset
///////////
path = fmt.Sprintf("/v2/assets/%d", 0)
path = fmt.Sprintf("/v2/assets/%d", expectedAssetIdx)
resp, data = makeRequest(t, listenAddr, path, false)
require.Equal(t, http.StatusOK, resp.StatusCode, fmt.Sprintf("unexpected return code, body: %s", string(data)))
var asset generated.AssetResponse
Expand Down
12 changes: 6 additions & 6 deletions api/handlers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestTransactionParamToTransactionFilter(t *testing.T) {
{
"Int field",
generated.SearchForTransactionsParams{AssetId: uint64Ptr(1234)},
idb.TransactionFilter{AssetID: 1234, Limit: defaultOpts.DefaultTransactionsLimit},
idb.TransactionFilter{AssetID: uint64Ptr(1234), Limit: defaultOpts.DefaultTransactionsLimit},
nil,
},
{
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestTransactionParamToTransactionFilter(t *testing.T) {
Round: nil,
MinRound: 2,
MaxRound: 3,
AssetID: 4,
AssetID: uint64Ptr(4),
BeforeTime: time.Date(2021, 1, 1, 1, 0, 0, 0, time.FixedZone("UTC", 0)),
AfterTime: time.Date(2022, 2, 2, 2, 0, 0, 0, time.FixedZone("UTC", 0)),
AlgosGT: nil,
Expand All @@ -134,7 +134,7 @@ func TestTransactionParamToTransactionFilter(t *testing.T) {
Offset: nil,
OffsetLT: nil,
OffsetGT: nil,
ApplicationID: 7,
ApplicationID: uint64Ptr(7),
},
nil,
},
Expand Down Expand Up @@ -195,7 +195,7 @@ func TestTransactionParamToTransactionFilter(t *testing.T) {
{
name: "Searching by application-id",
params: generated.SearchForTransactionsParams{ApplicationId: uint64Ptr(1234)},
filter: idb.TransactionFilter{ApplicationID: 1234, Limit: defaultOpts.DefaultTransactionsLimit},
filter: idb.TransactionFilter{ApplicationID: uint64Ptr(1234), Limit: defaultOpts.DefaultTransactionsLimit},
errorContains: nil,
},
{
Expand Down Expand Up @@ -299,14 +299,14 @@ func TestValidateTransactionFilter(t *testing.T) {
{
name: "application-id > math.MaxInt64",
filter: idb.TransactionFilter{
ApplicationID: math.MaxInt64 + 1,
ApplicationID: uint64Ptr(math.MaxInt64 + 1),
},
errorContains: []string{errValueExceedingInt64},
},
{
name: "asset-id > math.MaxInt64",
filter: idb.TransactionFilter{
AssetID: math.MaxInt64 + 1,
AssetID: uint64Ptr(math.MaxInt64 + 1),
},
errorContains: []string{errValueExceedingInt64},
},
Expand Down
6 changes: 3 additions & 3 deletions cmd/idbtest/idbtest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func doAssetQueryTests(db idb.IndexerDb) {
printAssetQuery(db, idb.AssetsQuery{Query: "us", Limit: 9})
printAssetQuery(db, idb.AssetsQuery{Name: "Tether USDt", Limit: 1})
printAssetQuery(db, idb.AssetsQuery{Unit: "USDt", Limit: 2})
printAssetQuery(db, idb.AssetsQuery{AssetID: 312769, Limit: 1})
printAssetQuery(db, idb.AssetsQuery{AssetIDGreaterThan: 312769, Query: "us", Limit: 2})
printAssetQuery(db, idb.AssetsQuery{AssetID: uint64Ptr(312769), Limit: 1})
printAssetQuery(db, idb.AssetsQuery{AssetIDGreaterThan: uint64Ptr(312769), Query: "us", Limit: 2})
tcreator, err := sdk_types.DecodeAddress("XIU7HGGAJ3QOTATPDSIIHPFVKMICXKHMOR2FJKHTVLII4FAOA3CYZQDLG4")
maybeFail(err, "addr decode, %v\n", err)
printAssetQuery(db, idb.AssetsQuery{Creator: tcreator[:], Limit: 1})
Expand Down Expand Up @@ -171,7 +171,7 @@ func main() {
printTxnQuery(db, idb.TransactionFilter{Limit: 2})
printTxnQuery(db, idb.TransactionFilter{MinRound: 5000000, Limit: 2})
printTxnQuery(db, idb.TransactionFilter{MaxRound: 100000, Limit: 2})
printTxnQuery(db, idb.TransactionFilter{AssetID: 604, Limit: 2})
printTxnQuery(db, idb.TransactionFilter{AssetID: uint64Ptr(604), Limit: 2})
printTxnQuery(db, idb.TransactionFilter{TypeEnum: 2, Limit: 2}) // keyreg
offset := uint64(3)
printTxnQuery(db, idb.TransactionFilter{Offset: &offset, Limit: 2})
Expand Down
16 changes: 8 additions & 8 deletions idb/idb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,11 @@ type TransactionFilter struct {
AlgosLT *uint64
RekeyTo *bool // nil for no filter

AssetID uint64 // filter transactions relevant to an asset
AssetID *uint64 // filter transactions relevant to an asset
AssetAmountGT *uint64
AssetAmountLT *uint64

ApplicationID uint64 // filter transactions relevant to an application
ApplicationID *uint64 // filter transactions relevant to an application

EffectiveAmountGT *uint64 // Algo: Amount + CloseAmount > x
EffectiveAmountLT *uint64 // Algo: Amount + CloseAmount < x
Expand Down Expand Up @@ -306,8 +306,8 @@ func (e MaxAPIResourcesPerAccountError) Error() string {

// AssetsQuery is a parameter object with all of the asset filter options.
type AssetsQuery struct {
AssetID uint64
AssetIDGreaterThan uint64
AssetID *uint64
AssetIDGreaterThan *uint64

Creator []byte

Expand Down Expand Up @@ -338,8 +338,8 @@ type AssetRow struct {

// AssetBalanceQuery is a parameter object with all of the asset balance filter options.
type AssetBalanceQuery struct {
AssetID uint64
AssetIDGT uint64
AssetID *uint64
AssetIDGT *uint64
AmountGT *uint64 // only rows > this
AmountLT *uint64 // only rows < this

Expand Down Expand Up @@ -376,8 +376,8 @@ type ApplicationRow struct {
// ApplicationQuery is a parameter object used for query local and global application state.
type ApplicationQuery struct {
Address []byte
ApplicationID uint64
ApplicationIDGreaterThan uint64
ApplicationID *uint64
ApplicationIDGreaterThan *uint64
IncludeDeleted bool
Limit uint64
}
Expand Down
Loading

0 comments on commit 316221e

Please sign in to comment.