diff --git a/api/handlers_e2e_test.go b/api/handlers_e2e_test.go index c70786b2..7612a746 100644 --- a/api/handlers_e2e_test.go +++ b/api/handlers_e2e_test.go @@ -2826,3 +2826,153 @@ func TestTxnSearchByGroupID(t *testing.T) { } } } + +// TestApplicationCreationZeroValueFilteringE2E tests application creation zero-value filtering +// end-to-end with real application transaction data. +func TestApplicationCreationZeroValueFilteringE2E(t *testing.T) { + db, shutdownFunc := setupIdb(t, test.MakeGenesis()) + defer shutdownFunc() + + // Load test data that contains application creation transactions + vb, err := test.ReadValidatedBlockFromFile("test_resources/validated_blocks/ApplicationHandlers.vb") + require.NoError(t, err) + err = db.AddBlock(&vb) + require.NoError(t, err) + + api := testServerImplementation(db) + + t.Run("application-id=0 should find app creation transactions", func(t *testing.T) { + // First, get all application transactions to establish baseline + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/v2/transactions?tx-type=appl", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + txType := generated.SearchForTransactionsParamsTxType("appl") + params := generated.SearchForTransactionsParams{TxType: &txType} + err := api.SearchForTransactions(c, params) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rec.Code) + + var allApplResponse generated.TransactionsResponse + err = json.Decode(rec.Body.Bytes(), &allApplResponse) + require.NoError(t, err) + + // Find transactions that have ApplicationID=0 (creation transactions) + var creationTransactions []generated.Transaction + for _, txn := range allApplResponse.Transactions { + if txn.ApplicationTransaction != nil && txn.ApplicationTransaction.ApplicationId == 0 { + creationTransactions = append(creationTransactions, txn) + } + } + + // Fail the test if we don't have creation transactions in the test data + // This ensures we're actually testing what we claim to test + require.Greater(t, len(creationTransactions), 0, + "Test data must contain application creation transactions to validate application-id=0 filtering") + + // Now test the zero-value filtering + req2 := httptest.NewRequest(http.MethodGet, "/v2/transactions?application-id=0", nil) + rec2 := httptest.NewRecorder() + c2 := e.NewContext(req2, rec2) + + applicationID := uint64(0) + params2 := generated.SearchForTransactionsParams{ApplicationId: &applicationID} + err = api.SearchForTransactions(c2, params2) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rec2.Code) + + var filteredResponse generated.TransactionsResponse + err = json.Decode(rec2.Body.Bytes(), &filteredResponse) + require.NoError(t, err) + + // After our fix, this should find the creation transactions + assert.Equal(t, len(creationTransactions), len(filteredResponse.Transactions), + "application-id=0 filter should return the same number of transactions as creation transactions found") + + // Verify all returned transactions are application creation transactions + for _, txn := range filteredResponse.Transactions { + require.NotNil(t, txn.ApplicationTransaction, "All filtered transactions should be application transactions") + assert.Equal(t, uint64(0), txn.ApplicationTransaction.ApplicationId, + "All filtered transactions should have ApplicationID=0 (creation transactions)") + } + }) +} + +// TestAssetCreationZeroValueFilteringE2E tests asset creation zero-value filtering +// end-to-end with real asset transaction data. +func TestAssetCreationZeroValueFilteringE2E(t *testing.T) { + db, shutdownFunc := setupIdb(t, test.MakeGenesis()) + defer shutdownFunc() + + // Load test data that contains asset creation transactions + vb, err := test.ReadValidatedBlockFromFile("test_resources/validated_blocks/AssetHoldings1.vb") + require.NoError(t, err) + err = db.AddBlock(&vb) + require.NoError(t, err) + + api := testServerImplementation(db) + + t.Run("asset-id=0 should find asset creation transactions", func(t *testing.T) { + // First, get all asset config transactions to see what we have + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/v2/transactions?tx-type=acfg", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + txType := generated.SearchForTransactionsParamsTxType("acfg") + params := generated.SearchForTransactionsParams{TxType: &txType} + err := api.SearchForTransactions(c, params) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rec.Code) + + var allAssetConfigResponse generated.TransactionsResponse + err = json.Decode(rec.Body.Bytes(), &allAssetConfigResponse) + require.NoError(t, err) + + // Look for asset creation transactions (where the transaction creates a new asset) + var assetCreationTransactions []generated.Transaction + for _, txn := range allAssetConfigResponse.Transactions { + if txn.AssetConfigTransaction != nil { + // Asset creation transactions have AssetId=0 and a non-zero CreatedAssetIndex + if txn.AssetConfigTransaction.AssetId != nil && *txn.AssetConfigTransaction.AssetId == 0 && + txn.CreatedAssetIndex != nil && *txn.CreatedAssetIndex > 0 { + assetCreationTransactions = append(assetCreationTransactions, txn) + } + } + } + + // Fail the test if we don't have asset creation transactions in the test data + // This ensures we're actually testing what we claim to test + require.Greater(t, len(assetCreationTransactions), 0, + "Test data must contain asset creation transactions to validate asset-id=0 filtering") + + // Now test the zero-value filtering + req2 := httptest.NewRequest(http.MethodGet, "/v2/transactions?asset-id=0", nil) + rec2 := httptest.NewRecorder() + c2 := e.NewContext(req2, rec2) + + assetID := uint64(0) + params2 := generated.SearchForTransactionsParams{AssetId: &assetID} + err = api.SearchForTransactions(c2, params2) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rec2.Code) + + var filteredResponse generated.TransactionsResponse + err = json.Decode(rec2.Body.Bytes(), &filteredResponse) + require.NoError(t, err) + + // After our fix, this should find the creation transactions + assert.Equal(t, len(assetCreationTransactions), len(filteredResponse.Transactions), + "asset-id=0 filter should return the same number of transactions as asset creation transactions found") + + // Verify all returned transactions are asset creation transactions + for _, txn := range filteredResponse.Transactions { + if txn.AssetConfigTransaction != nil { + require.NotNil(t, txn.AssetConfigTransaction.AssetId, "AssetConfigTransaction.AssetId should not be nil") + assert.Equal(t, uint64(0), *txn.AssetConfigTransaction.AssetId, + "All filtered transactions should have AssetId=0 (creation transactions)") + } + } + }) +} diff --git a/idb/postgres/postgres.go b/idb/postgres/postgres.go index 348c05d3..454b0733 100644 --- a/idb/postgres/postgres.go +++ b/idb/postgres/postgres.go @@ -590,27 +590,75 @@ func buildTransactionQuery(tf idb.TransactionFilter) (query string, whereArgs [] } if tf.AssetID != nil || tf.ApplicationID != nil { var creatableID uint64 + var isApplicationID bool if tf.AssetID != nil { creatableID = *tf.AssetID + isApplicationID = false if tf.ApplicationID != nil { if *tf.AssetID != *tf.ApplicationID { return "", nil, fmt.Errorf("cannot search both assetid and appid") } + isApplicationID = true // Both are set with same value } } else { creatableID = *tf.ApplicationID + isApplicationID = true + } + + // For zero values, we need to query the original JSON field to find creation transactions + // For non-zero values, we can use the optimized t.asset column + if creatableID == 0 { + if isApplicationID { + // ApplicationID=0 means application creation - query the original field + // We also need to restrict to application call transactions + whereParts = append(whereParts, fmt.Sprintf("t.typeenum = $%d", partNumber)) + whereArgs = append(whereArgs, int(idb.TypeEnumApplication)) + partNumber++ + + // For application creation, the apid field might be 0, null, or missing + // We check for all these cases + whereParts = append(whereParts, fmt.Sprintf("((t.txn -> 'txn' -> 'apid')::bigint = $%d OR (t.txn -> 'txn' -> 'apid') IS NULL)", partNumber)) + whereArgs = append(whereArgs, creatableID) + partNumber++ + } else { + // AssetID=0 can mean: + // 1. Asset transfers of Algos (XferAsset=0) - these work with t.asset=0 + // 2. Asset creation (ConfigAsset=0) - these need original field query + // We need to handle both cases with an OR condition + whereParts = append(whereParts, fmt.Sprintf("(t.asset = $%d OR (t.typeenum = $%d AND ((t.txn -> 'txn' -> 'caid')::bigint = $%d OR (t.txn -> 'txn' -> 'caid') IS NULL)))", partNumber, partNumber+1, partNumber+2)) + whereArgs = append(whereArgs, creatableID) // For asset transfers (XferAsset=0) + whereArgs = append(whereArgs, int(idb.TypeEnumAssetConfig)) // Asset creation transactions + whereArgs = append(whereArgs, creatableID) // For asset creation (ConfigAsset=0) + partNumber += 3 + } + } else { + // Non-zero values: continue using the optimized t.asset column + whereParts = append(whereParts, fmt.Sprintf("t.asset = $%d", partNumber)) + whereArgs = append(whereArgs, creatableID) + partNumber++ } - whereParts = append(whereParts, fmt.Sprintf("t.asset = $%d", partNumber)) - whereArgs = append(whereArgs, creatableID) - partNumber++ } if tf.AssetAmountGT != nil { - whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'aamt')::numeric(20) > $%d", partNumber)) + // Handle asset amount filtering with proper field selection and NULL handling + if tf.AssetID != nil && *tf.AssetID == 0 { + // For Algo transfers (asset-id=0), use the 'amt' field with COALESCE + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'amt')::bigint, 0) > $%d", partNumber)) + } else { + // For asset transfers, use the 'aamt' field with COALESCE + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'aamt')::numeric(20), 0) > $%d", partNumber)) + } whereArgs = append(whereArgs, *tf.AssetAmountGT) partNumber++ } if tf.AssetAmountLT != nil { - whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'aamt')::numeric(20) < $%d", partNumber)) + // Handle asset amount filtering with proper field selection and NULL handling + if tf.AssetID != nil && *tf.AssetID == 0 { + // For Algo transfers (asset-id=0), use the 'amt' field with COALESCE + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'amt')::bigint, 0) < $%d", partNumber)) + } else { + // For asset transfers, use the 'aamt' field with COALESCE + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'aamt')::numeric(20), 0) < $%d", partNumber)) + } whereArgs = append(whereArgs, *tf.AssetAmountLT) partNumber++ } @@ -660,12 +708,14 @@ func buildTransactionQuery(tf idb.TransactionFilter) (query string, whereArgs [] partNumber++ } if tf.AlgosGT != nil { - whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'amt')::bigint > $%d", partNumber)) + // Handle Algo amount filtering with COALESCE for NULL values + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'amt')::bigint, 0) > $%d", partNumber)) whereArgs = append(whereArgs, *tf.AlgosGT) partNumber++ } if tf.AlgosLT != nil { - whereParts = append(whereParts, fmt.Sprintf("(t.txn -> 'txn' -> 'amt')::bigint < $%d", partNumber)) + // Handle Algo amount filtering with COALESCE for NULL values + whereParts = append(whereParts, fmt.Sprintf("COALESCE((t.txn -> 'txn' -> 'amt')::bigint, 0) < $%d", partNumber)) whereArgs = append(whereArgs, *tf.AlgosLT) partNumber++ } diff --git a/idb/postgres/postgres_test.go b/idb/postgres/postgres_test.go index 71a9d50e..378cc8eb 100644 --- a/idb/postgres/postgres_test.go +++ b/idb/postgres/postgres_test.go @@ -157,3 +157,158 @@ func Test_buildTransactionQueryApplicationLogs(t *testing.T) { }) } } + +// Test_buildTransactionQueryZeroValues tests the SQL generation for zero-value filters. +// This test verifies the custom behavior for application-id=0 and asset-id=0 filtering. +func Test_buildTransactionQueryZeroValues(t *testing.T) { + tests := []struct { + name string + filter idb.TransactionFilter + expectedSQL []string // SQL fragments that should be present + notExpectedSQL []string // SQL fragments that should NOT be present + expectedArgs []interface{} + description string + }{ + { + name: "ApplicationID zero - query original JSON field for app creation", + filter: idb.TransactionFilter{ + ApplicationID: uint64Ptr(0), + Limit: 10, + }, + expectedSQL: []string{"t.typeenum = ", "t.txn -> 'txn' -> 'apid'", "IS NULL"}, + notExpectedSQL: []string{}, + expectedArgs: []interface{}{int(idb.TypeEnumApplication), uint64(0)}, + description: "should query original JSON field for application creation transactions", + }, + { + name: "ApplicationID non-zero - should query t.asset", + filter: idb.TransactionFilter{ + ApplicationID: uint64Ptr(123), + Limit: 10, + }, + expectedSQL: []string{"t.asset = "}, + notExpectedSQL: []string{"t.txn -> 'txn' -> 'apid'"}, + expectedArgs: []interface{}{uint64(123)}, + description: "Non-zero ApplicationID should use t.asset column", + }, + { + name: "AssetID zero - should work for both transfers and creation", + filter: idb.TransactionFilter{ + AssetID: uint64Ptr(0), + Limit: 10, + }, + expectedSQL: []string{"t.asset = ", "t.typeenum = ", "t.txn -> 'txn' -> 'caid'", "IS NULL"}, + notExpectedSQL: []string{}, + expectedArgs: []interface{}{uint64(0), int(idb.TypeEnumAssetConfig), uint64(0)}, + description: "AssetID=0 should work for both Algo transfers (t.asset=0) and asset creation (original JSON field)", + }, + { + name: "AssetID non-zero - should query t.asset", + filter: idb.TransactionFilter{ + AssetID: uint64Ptr(456), + Limit: 10, + }, + expectedSQL: []string{"t.asset = "}, + notExpectedSQL: []string{}, + expectedArgs: []interface{}{uint64(456)}, + description: "Non-zero AssetID should use t.asset column", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + query, whereArgs, err := buildTransactionQuery(tt.filter) + require.NoError(t, err, "buildTransactionQuery should not error: %s", tt.description) + + // Check that expected SQL fragments are present + for _, expectedFragment := range tt.expectedSQL { + assert.Contains(t, query, expectedFragment, + "Query should contain '%s': %s", expectedFragment, tt.description) + } + + // Check that unexpected SQL fragments are NOT present + for _, notExpectedFragment := range tt.notExpectedSQL { + assert.NotContains(t, query, notExpectedFragment, + "Query should NOT contain '%s': %s", notExpectedFragment, tt.description) + } + + // Check that the arguments match expected values + if len(tt.expectedArgs) > 0 { + // Note: whereArgs may have more arguments than we're testing, + // so we just check that our expected args are somewhere in there + foundExpectedArgs := 0 + for _, expectedArg := range tt.expectedArgs { + for _, actualArg := range whereArgs { + if actualArg == expectedArg { + foundExpectedArgs++ + break + } + } + } + assert.Equal(t, len(tt.expectedArgs), foundExpectedArgs, + "Should find all expected arguments in whereArgs: %s", tt.description) + } + + }) + } +} + +// Test_buildTransactionQueryCurrencyLessThanZeroValues tests currency-less-than filtering +// scenarios that should include zero-value transactions but may be affected by NULL handling. +func Test_buildTransactionQueryCurrencyLessThanZeroValues(t *testing.T) { + t.Run("currency-less-than=1 with payment transactions should include zero amounts", func(t *testing.T) { + filter := idb.TransactionFilter{ + AlgosLT: uint64Ptr(1), + TypeEnum: idb.TypeEnumPay, + Limit: 10, + } + + query, _, err := buildTransactionQuery(filter) + require.NoError(t, err) + + // Should generate SQL that handles both existing and NULL amt fields + // After fix: should use COALESCE to treat NULL as 0 + assert.Contains(t, query, "t.txn -> 'txn' -> 'amt'", + "Should query the amt field for payment transactions") + + // Note: This test documents current behavior. After the fix, it should use COALESCE. + // The important thing is that zero-amount payments should be included in results. + }) + + t.Run("currency-less-than=1 with tx-type=axfer should include zero-amount transfers", func(t *testing.T) { + filter := idb.TransactionFilter{ + AssetAmountLT: uint64Ptr(1), + TypeEnum: idb.TypeEnumAssetTransfer, + Limit: 10, + } + + query, _, err := buildTransactionQuery(filter) + require.NoError(t, err) + + // Should handle both asset transfers (aamt) and Algo transfers (amt) + // After fix: should query both fields or use appropriate field based on context + assert.Contains(t, query, "t.txn -> 'txn' -> 'aamt'", + "Should query asset amount field for asset transfers") + + // Note: This test documents the challenge - asset transfers include both + // actual asset transfers (use aamt) and Algo transfers (use amt) + }) + + t.Run("currency-less-than=1 with asset-id=0 should include zero-amount Algo transfers", func(t *testing.T) { + filter := idb.TransactionFilter{ + AssetAmountLT: uint64Ptr(1), + AssetID: uint64Ptr(0), // Algo transfers + Limit: 10, + } + + query, _, err := buildTransactionQuery(filter) + require.NoError(t, err) + + // Should use the correct field for Algo transfers (amt, not aamt) + // After fix: should query amt field when asset-id=0 + + // This scenario specifically tests Algo transfers which store amount in 'amt' field + assert.Contains(t, query, "t.txn -> 'txn' -> 'amt'", + "Should query amt field for Algo transfers (asset-id=0)") + }) +}