Repository: algorand/indexer (go)

Issue:
Query parameters after-time and before-time for /v2/transactions always assume UTC even if a different timezone is specified
## Summary

Query parameters after-time and before-time for /v2/transactions always assume UTC even if a different timezone is specified.

```bash
$ curl -s "https://testnet.algoexplorerapi.io/idx2/v2/transactions?after-time=2021-04-24T21:30:00-04:00&before-time=2021-04-24T21:30:08-04:00" | jq
{
  "current-round": 13748828,
  "next-token": "9LrRAAAAAAAAAAAA",
  "transactions": [
    {
      "asset-transfer-transaction": {
        "amount": 100000000,
        "asset-id": 12726651,
        "close-amount": 0,
        "receiver": "YM7QN2ORZGARPPMBPMZZMLLJDHSVPZU75HUPMRQUEUU6VQOXADQJ4HUIQM"
      },
      "close-rewards": 0,
      "closing-amount": 0,
      "confirmed-round": 13744884,
      "fee": 1000,
      "first-valid": 13744882,
      "genesis-hash": "SGO1GKSzyE7IEPItTxCByw9x8FmnrCDexi9/cOUJOiI=",
      "genesis-id": "testnet-v1.0",
      "id": "LGM5ZPNGECOXVKQOBUVVF7FX4DIAE24ZDAXWOUCP5ORWX5VTQSFA",
      "intra-round-offset": 0,
      "last-valid": 13745882,
      "note": "gqZwVHhuSWTZJDcwNmVlZGVmLWI0OTctNDcwMi1iYzcwLWNjNTdkYzRjZTIzNKR0eXBlpWlzc3Vl",
      "receiver-rewards": 0,
      "round-time": 1619299805,
      "sender": "UPCPV3UZ4GKXERPFSKDZ3ZRJRN3AFKZYXU7CIENQQ6E4KOWEXRUJ7KOHUQ",
      "sender-rewards": 439,
      "signature": {
        "sig": "YsqUETiATb7n/AauJ0zCdZxPn2WVGpGcAYbJgXD35DCCJ5ZNdg5ZbTsvimnxdMRN2HvKTWJYQJZlimBSi+aSBg=="
      },
      "tx-type": "axfer"
    }
  ]
}
```
where the round timestamp is 1619299805, that is 2021-04-24 17:30:05-04:00.

This is the same result as:
```bash
$ curl -s "https://testnet.algoexplorerapi.io/idx2/v2/transactions?after-time=2021-04-24T21:30:00Z&before-time=2021-04-24T21:30:08Z" | jq
```

Same issue with PureStake API indexer endpoint.

## Scope/Requirements

Support for timezone or require explicitly parameters to be in the UTC timezone and fail otherwise.

## Urgency
Since you can always convert the time you get, this isn't super urgent. However, this is a bug that we should fix.

Pull request:
api: Convert timezones before txn query

## Summary

Fixes #458 

## Test Plan

Added unit tests.

Patch:
diff --git a/api/converter_utils.go b/api/converter_utils.go
index 6f483bf..c486b72 100644
--- a/api/converter_utils.go
+++ b/api/converter_utils.go
@@ -328,26 +328,29 @@ func signedTxnWithAdToTransaction(stxn *sdk.SignedTxnWithAD, extra rowData) (gen
 		}
 		keyreg = &k
 	case sdk.AssetConfigTx:
-		assetParams := generated.AssetParams{
-			Clawback:      addrPtr(stxn.Txn.AssetParams.Clawback),
-			Creator:       stxn.Txn.Sender.String(),
-			Decimals:      uint64(stxn.Txn.AssetParams.Decimals),
-			DefaultFrozen: boolPtr(stxn.Txn.AssetParams.DefaultFrozen),
-			Freeze:        addrPtr(stxn.Txn.AssetParams.Freeze),
-			Manager:       addrPtr(stxn.Txn.AssetParams.Manager),
-			MetadataHash:  byteSliceOmitZeroPtr(stxn.Txn.AssetParams.MetadataHash[:]),
-			Name:          strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.AssetName)),
-			NameB64:       byteSlicePtr([]byte(stxn.Txn.AssetParams.AssetName)),
-			Reserve:       addrPtr(stxn.Txn.AssetParams.Reserve),
-			Total:         stxn.Txn.AssetParams.Total,
-			UnitName:      strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.UnitName)),
-			UnitNameB64:   byteSlicePtr([]byte(stxn.Txn.AssetParams.UnitName)),
-			Url:           strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.URL)),
-			UrlB64:        byteSlicePtr([]byte(stxn.Txn.AssetParams.URL)),
+		var assetParams *generated.AssetParams = nil
+		if !stxn.Txn.AssetParams.IsZero() {
+			assetParams = &generated.AssetParams{
+				Clawback:      addrPtr(stxn.Txn.AssetParams.Clawback),
+				Creator:       stxn.Txn.Sender.String(),
+				Decimals:      uint64(stxn.Txn.AssetParams.Decimals),
+				DefaultFrozen: boolPtr(stxn.Txn.AssetParams.DefaultFrozen),
+				Freeze:        addrPtr(stxn.Txn.AssetParams.Freeze),
+				Manager:       addrPtr(stxn.Txn.AssetParams.Manager),
+				MetadataHash:  byteSliceOmitZeroPtr(stxn.Txn.AssetParams.MetadataHash[:]),
+				Name:          strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.AssetName)),
+				NameB64:       byteSlicePtr([]byte(stxn.Txn.AssetParams.AssetName)),
+				Reserve:       addrPtr(stxn.Txn.AssetParams.Reserve),
+				Total:         stxn.Txn.AssetParams.Total,
+				UnitName:      strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.UnitName)),
+				UnitNameB64:   byteSlicePtr([]byte(stxn.Txn.AssetParams.UnitName)),
+				Url:           strPtr(util.PrintableUTF8OrEmpty(stxn.Txn.AssetParams.URL)),
+				UrlB64:        byteSlicePtr([]byte(stxn.Txn.AssetParams.URL)),
+			}
 		}
 		config := generated.TransactionAssetConfig{
 			AssetId: uint64Ptr(uint64(stxn.Txn.ConfigAsset)),
-			Params:  &assetParams,
+			Params:  assetParams,
 		}
 		assetConfig = &config
 	case sdk.AssetTransferTx:
diff --git a/idb/postgres/postgres.go b/idb/postgres/postgres.go
index e0161b7..21594c4 100644
--- a/idb/postgres/postgres.go
+++ b/idb/postgres/postgres.go
@@ -564,13 +564,15 @@ func buildTransactionQuery(tf idb.TransactionFilter) (query string, whereArgs []
 		partNumber++
 	}
 	if !tf.BeforeTime.IsZero() {
+		convertedTime := tf.BeforeTime.In(time.UTC)
 		whereParts = append(whereParts, fmt.Sprintf("h.realtime < $%d", partNumber))
-		whereArgs = append(whereArgs, tf.BeforeTime)
+		whereArgs = append(whereArgs, convertedTime)
 		partNumber++
 	}
 	if !tf.AfterTime.IsZero() {
+		convertedTime := tf.AfterTime.In(time.UTC)
 		whereParts = append(whereParts, fmt.Sprintf("h.realtime > $%d", partNumber))
-		whereArgs = append(whereArgs, tf.AfterTime)
+		whereArgs = append(whereArgs, convertedTime)
 		partNumber++
 	}
 	if tf.AssetID != 0 || tf.ApplicationID != 0 {


Test patch:
diff --git a/api/handlers_e2e_test.go b/api/handlers_e2e_test.go
index 1fb3aaf..07fd8a1 100644
--- a/api/handlers_e2e_test.go
+++ b/api/handlers_e2e_test.go
@@ -2305,13 +2305,31 @@ func TestAssetDelete(t *testing.T) {
 		})
 	}
 
+	// Look up apars to make sure they're not there
+	resp, data := makeRequest(t, listenAddr, fmt.Sprintf("/v2/blocks/%d", vb.Block.Round), false)
+	require.Equal(t, http.StatusOK, resp.StatusCode, fmt.Sprintf("unexpected return code for blocks endpoint, body: %s", string(data)))
+	var blk generated.BlockResponse
+	err = json.Decode(data, &blk)
+	require.NoError(t, err)
+	assert.NotNil(t, blk.Transactions)
+	blkTxns := *blk.Transactions
+	assert.Equal(t, 3, len(blkTxns))
+	// Create Txn -- non-nil apars
+	assert.NotNil(t, blkTxns[0].AssetConfigTransaction)
+	blkAssetCfgCreate := *blkTxns[0].AssetConfigTransaction
+	assert.NotNil(t, blkAssetCfgCreate.Params)
+	// Delete Txn -- nil apars
+	assert.NotNil(t, blkTxns[2].AssetConfigTransaction)
+	blkAssetCfgDelete := *blkTxns[2].AssetConfigTransaction
+	assert.Nil(t, blkAssetCfgDelete.Params)
+
 	//////////
 	// And // look up the accounts opted in to the asset
 	//////////
 
 	// deleted asset excluded
 	path := fmt.Sprintf("/v2/accounts/%s", test.AccountA)
-	resp, data := makeRequest(t, listenAddr, path, false)
+	resp, data = makeRequest(t, listenAddr, path, false)
 	require.Equal(t, http.StatusOK, resp.StatusCode, fmt.Sprintf("unexpected return code, body: %s", string(data)))
 	var account generated.AccountResponse
 	err = json.Decode(data, &account)
diff --git a/idb/postgres/postgres_integration_test.go b/idb/postgres/postgres_integration_test.go
index e60536f..5136912 100644
--- a/idb/postgres/postgres_integration_test.go
+++ b/idb/postgres/postgres_integration_test.go
@@ -314,6 +314,58 @@ func TestMultipleWriters(t *testing.T) {
 	assert.Equal(t, amt, balance)
 }
 
+// TestTransactionsTimestamps tests that the transactions endpoint responds to times properly.
+func TestTransactionsTimestamps(t *testing.T) {
+	db, shutdownFunc := setupIdb(t, test.MakeGenesis())
+	defer shutdownFunc()
+
+	round := uint64(1)
+	///////////
+	// Given // A block with 8 transactions at ts 1671036853.
+	///////////
+	usEastTz, err := time.LoadLocation("America/New_York")
+	require.NoError(t, err)
+	usWestTz, err := time.LoadLocation("America/Los_Angeles")
+	require.NoError(t, err)
+	vb, err := test.ReadValidatedBlockFromFile("test_resources/validated_blocks/BlockWithTransactions.vb")
+	require.NoError(t, err)
+	err = db.AddBlock(&vb)
+	require.NoError(t, err)
+
+	//////////
+	// When // We call Transactions with timestamp filters
+	//////////
+	blkTime := vb.Block.BlockHeader.TimeStamp
+	fullRowsCh, _ := db.Transactions(
+		context.Background(),
+		idb.TransactionFilter{
+			Round:      &round,
+			BeforeTime: time.Unix(blkTime+1, 0).In(usEastTz),
+			AfterTime:  time.Unix(blkTime-1, 0)})
+	var txnRows0 []idb.TxnRow
+	for row := range fullRowsCh {
+		require.NoError(t, row.Error)
+		txnRows0 = append(txnRows0, row)
+	}
+
+	emptyRowsCh, _ := db.Transactions(
+		context.Background(),
+		idb.TransactionFilter{
+			Round:     &round,
+			AfterTime: time.Unix(blkTime, 0).In(usWestTz)})
+	txnRows1 := make([]idb.TxnRow, 0)
+	for row := range emptyRowsCh {
+		require.NoError(t, row.Error)
+		txnRows1 = append(txnRows1, row)
+	}
+
+	//////////
+	// Then // They should have the correct number of transactions
+	//////////
+	assert.Len(t, txnRows0, len(vb.Block.Payset))
+	assert.Len(t, txnRows1, 0)
+}
+
 // TestBlockWithTransactions tests that the block with transactions endpoint works.
 func TestBlockWithTransactions(t *testing.T) {
 	db, shutdownFunc := setupIdb(t, test.MakeGenesis())
diff --git a/idb/postgres/postgres_test.go b/idb/postgres/postgres_test.go
index 4b91fec..43b731b 100644
--- a/idb/postgres/postgres_test.go
+++ b/idb/postgres/postgres_test.go
@@ -74,3 +74,48 @@ func Test_UnknownProtocol(t *testing.T) {
 	require.ErrorContains(t, err, protocol)
 	require.ErrorContains(t, err, "you need to upgrade")
 }
+
+func Test_buildTransactionQueryTime(t *testing.T) {
+	us_east_tz, err := time.LoadLocation("America/New_York")
+	require.NoError(t, err)
+	us_west_tz, err := time.LoadLocation("America/Los_Angeles")
+	require.NoError(t, err)
+	random_date_utc := time.Date(1000, time.December, 25, 1, 2, 3, 4, time.UTC)
+	tests := []struct {
+		name      string
+		arg       idb.TransactionFilter
+		whereArgs []interface{}
+	}{
+		{
+			"BeforeTime UTC to UTC",
+			idb.TransactionFilter{
+				BeforeTime: random_date_utc,
+			},
+			[]interface{}{random_date_utc},
+		},
+		{
+			"AfterTime UTC to UTC",
+			idb.TransactionFilter{
+				AfterTime: random_date_utc,
+			},
+			[]interface{}{random_date_utc},
+		},
+		{
+			"BeforeTime AfterTime Conversion",
+			idb.TransactionFilter{
+				BeforeTime: time.Date(1000, time.December, 25, 1, 2, 3, 4, us_east_tz),
+				AfterTime:  time.Date(1000, time.December, 25, 1, 2, 3, 4, us_west_tz),
+			},
+			[]interface{}{
+				time.Date(1000, time.December, 25, 1, 2, 3, 4, us_east_tz).UTC(),
+				time.Date(1000, time.December, 25, 1, 2, 3, 4, us_west_tz).UTC(),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, whereArgs, _ := buildTransactionQuery(tt.arg)
+			require.Equal(t, whereArgs, tt.whereArgs)
+		})
+	}
+}