From 2a2d9cb818a7cb52109c312877a63334a55ef4a1 Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Thu, 4 Nov 2021 14:42:19 -0400 Subject: [PATCH 1/6] Write zero address in txn_participation table. --- accounting/accounting.go | 29 +++++++++++++++++++------- idb/postgres/internal/writer/writer.go | 6 ------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/accounting/accounting.go b/accounting/accounting.go index de38681f9..d0b87f5bc 100644 --- a/accounting/accounting.go +++ b/accounting/accounting.go @@ -3,6 +3,7 @@ package accounting import ( "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/transactions" + "github.com/algorand/go-algorand/protocol" ) // GetTransactionParticipants calls function `add` for every address referenced in the @@ -11,15 +12,27 @@ func GetTransactionParticipants(stxnad *transactions.SignedTxnWithAD, includeInn txn := &stxnad.Txn add(txn.Sender) - add(txn.Receiver) - add(txn.CloseRemainderTo) - add(txn.AssetSender) - add(txn.AssetReceiver) - add(txn.AssetCloseTo) - add(txn.FreezeAccount) - for _, address := range txn.ApplicationCallTxnFields.Accounts { - add(address) + switch txn.Type { + case protocol.PaymentTx: + add(txn.Receiver) + if !txn.CloseRemainderTo.IsZero() { + add(txn.CloseRemainderTo) + } + case protocol.AssetTransferTx: + if !txn.AssetSender.IsZero() { + add(txn.AssetSender) + } + add(txn.AssetReceiver) + if !txn.AssetCloseTo.IsZero() { + add(txn.AssetCloseTo) + } + case protocol.AssetFreezeTx: + add(txn.FreezeAccount) + case protocol.ApplicationCallTx: + for _, address := range txn.ApplicationCallTxnFields.Accounts { + add(address) + } } if includeInner { diff --git a/idb/postgres/internal/writer/writer.go b/idb/postgres/internal/writer/writer.go index 582890ceb..638de664f 100644 --- a/idb/postgres/internal/writer/writer.go +++ b/idb/postgres/internal/writer/writer.go @@ -274,9 +274,6 @@ func getTransactionParticipants(stxnad *transactions.SignedTxnWithAD, includeInn // if no inner transactions then adding into a slice with in-place de-duplication res := make([]basics.Address, 0, acctsPerTxn) add := func(address basics.Address) { - if address.IsZero() { - return - } for _, p := range res { if address == p { return @@ -295,9 +292,6 @@ func getTransactionParticipants(stxnad *transactions.SignedTxnWithAD, includeInn size := acctsPerTxn * (1 + len(stxnad.ApplyData.EvalDelta.InnerTxns)) // approx participants := make(map[basics.Address]struct{}, size) add := func(address basics.Address) { - if address.IsZero() { - return - } participants[address] = struct{}{} } From 2d47eb81899d884059ab9ee9093ac19d18c5790b Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Thu, 4 Nov 2021 21:02:23 -0400 Subject: [PATCH 2/6] Add comments. --- accounting/accounting.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/accounting/accounting.go b/accounting/accounting.go index d0b87f5bc..63c860068 100644 --- a/accounting/accounting.go +++ b/accounting/accounting.go @@ -16,14 +16,18 @@ func GetTransactionParticipants(stxnad *transactions.SignedTxnWithAD, includeInn switch txn.Type { case protocol.PaymentTx: add(txn.Receiver) + // Close address is optional. if !txn.CloseRemainderTo.IsZero() { add(txn.CloseRemainderTo) } case protocol.AssetTransferTx: + // If asset sender is non-zero, it is a clawback transaction. Otherwise, + // the transaction sender address is used. if !txn.AssetSender.IsZero() { add(txn.AssetSender) } add(txn.AssetReceiver) + // Asset close address is optional. if !txn.AssetCloseTo.IsZero() { add(txn.AssetCloseTo) } From c563e9927208de5a93a1cf1fd20f799f02697ca1 Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Mon, 8 Nov 2021 12:51:47 -0500 Subject: [PATCH 3/6] Add a restriction. --- api/handlers.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/api/handlers.go b/api/handlers.go index 10f8e7bde..b8c27f53c 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -487,6 +487,15 @@ func (si *ServerImplementation) SearchForTransactions(ctx echo.Context, params g return badRequest(ctx, err.Error()) } + if filter.AddressRole != 0 { + var address basics.Address + copy(address[:], filter.Address) + if address.IsZero() { + return badRequest( + ctx, "searching transactions by zero address with address role is not supported") + } + } + // Fetch the transactions txns, next, round, err := si.fetchTransactions(ctx.Request().Context(), filter) if err != nil { From f7c7f68bcd066443ea2d8c8fa0c5911e89abdaa7 Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Mon, 8 Nov 2021 15:01:59 -0500 Subject: [PATCH 4/6] Move error msg to error_messages.go. --- api/error_messages.go | 1 + api/handlers.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/error_messages.go b/api/error_messages.go index 82c0d0762..109818f64 100644 --- a/api/error_messages.go +++ b/api/error_messages.go @@ -29,6 +29,7 @@ const ( errTransactionSearch = "error while searching for transaction" errSpecialAccounts = "indexer doesn't support fee sink and rewards pool accounts, please refer to algod for relevant information" errFailedLoadSpecialAccounts = "failed to retrieve special accounts" + errZeroAddressRole = "searching transactions by zero address with address role is not supported" ) var errUnknownAddressRole string diff --git a/api/handlers.go b/api/handlers.go index b8c27f53c..71bb1de13 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -491,8 +491,7 @@ func (si *ServerImplementation) SearchForTransactions(ctx echo.Context, params g var address basics.Address copy(address[:], filter.Address) if address.IsZero() { - return badRequest( - ctx, "searching transactions by zero address with address role is not supported") + return badRequest(ctx, errZeroAddressRole) } } From 0c6c56a19453d675c3d6667668f3cc37a26800e3 Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Mon, 8 Nov 2021 17:55:29 -0500 Subject: [PATCH 5/6] Take out parameter validation out of transactionParamsToTransactionFilter(). --- api/converter_utils.go | 10 ----- api/error_messages.go | 4 +- api/handlers.go | 62 ++++++++++++++++++++++++---- api/handlers_test.go | 92 +++++++++++++++++++++++++++++++++--------- api/pointer_utils.go | 6 +++ 5 files changed, 136 insertions(+), 38 deletions(-) diff --git a/api/converter_utils.go b/api/converter_utils.go index 5648d9128..26b519c75 100644 --- a/api/converter_utils.go +++ b/api/converter_utils.go @@ -556,16 +556,6 @@ func assetParamsToAssetQuery(params generated.SearchForAssetsParams) (idb.Assets func transactionParamsToTransactionFilter(params generated.SearchForTransactionsParams) (filter idb.TransactionFilter, err error) { var errorArr = make([]string, 0) - // Round + min/max round - if params.Round != nil && (params.MaxRound != nil || params.MinRound != nil) { - errorArr = append(errorArr, errInvalidRoundAndMinMax) - } - - // If min/max are mixed up - if params.Round == nil && params.MinRound != nil && params.MaxRound != nil && *params.MinRound > *params.MaxRound { - errorArr = append(errorArr, errInvalidRoundMinMax) - } - // Integer filter.MaxRound = uintOrDefault(params.MaxRound) filter.MinRound = uintOrDefault(params.MinRound) diff --git a/api/error_messages.go b/api/error_messages.go index 109818f64..e9610cf11 100644 --- a/api/error_messages.go +++ b/api/error_messages.go @@ -29,7 +29,9 @@ const ( errTransactionSearch = "error while searching for transaction" errSpecialAccounts = "indexer doesn't support fee sink and rewards pool accounts, please refer to algod for relevant information" errFailedLoadSpecialAccounts = "failed to retrieve special accounts" - errZeroAddressRole = "searching transactions by zero address with address role is not supported" + errZeroAddressCloseRemainderToRole = "searching transactions by zero address with close address role is not supported" + errZeroAddressAssetSenderRole = "searching transactions by zero address with asset sender role is not supported" + errZeroAddressAssetCloseToRole = "searching transactions by zero address with asset close address role is not supported" ) var errUnknownAddressRole string diff --git a/api/handlers.go b/api/handlers.go index 71bb1de13..33d168da4 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strconv" + "strings" "github.com/algorand/go-algorand/data/basics" "github.com/labstack/echo/v4" @@ -52,6 +53,46 @@ const defaultAssetsLimit = 100 const maxBalancesLimit = 10000 const defaultBalancesLimit = 1000 +////////////////////// +// Helper functions // +////////////////////// + +func validateTransactionFilter(filter *idb.TransactionFilter) error { + var errorArr = make([]string, 0) + + // Round + min/max round + if filter.Round != nil && (filter.MaxRound != 0 || filter.MinRound != 0) { + errorArr = append(errorArr, errInvalidRoundAndMinMax) + } + + // If min/max are mixed up + if filter.Round == nil && filter.MinRound != 0 && filter.MaxRound != 0 && filter.MinRound > filter.MaxRound { + errorArr = append(errorArr, errInvalidRoundMinMax) + } + + { + var address basics.Address + copy(address[:], filter.Address) + if address.IsZero() { + if filter.AddressRole & idb.AddressRoleCloseRemainderTo != 0 { + errorArr = append(errorArr, errZeroAddressCloseRemainderToRole) + } + if filter.AddressRole & idb.AddressRoleAssetSender != 0 { + errorArr = append(errorArr, errZeroAddressAssetSenderRole) + } + if filter.AddressRole & idb.AddressRoleAssetCloseTo != 0 { + errorArr = append(errorArr, errZeroAddressAssetCloseToRole) + } + } + } + + if len(errorArr) > 0 { + return errors.New("invalid input: " + strings.Join(errorArr, ", ")) + } + + return nil +} + //////////////////////////// // Handler implementation // //////////////////////////// @@ -274,13 +315,18 @@ func (si *ServerImplementation) LookupApplicationLogsByID(ctx echo.Context, appl MinRound: params.MinRound, MaxRound: params.MaxRound, Address: params.SenderAddress, - AddressRole: strPtr(addrRoleSender), } filter, err := transactionParamsToTransactionFilter(searchParams) if err != nil { return badRequest(ctx, err.Error()) } + filter.AddressRole = idb.AddressRoleSender + + err = validateTransactionFilter(&filter) + if err != nil { + return badRequest(ctx, err.Error()) + } // Fetch the transactions txns, next, round, err := si.fetchTransactions(ctx.Request().Context(), filter) @@ -457,6 +503,11 @@ func (si *ServerImplementation) LookupTransaction(ctx echo.Context, txid string) return badRequest(ctx, err.Error()) } + err = validateTransactionFilter(&filter) + if err != nil { + return badRequest(ctx, err.Error()) + } + // Fetch the transactions txns, _, round, err := si.fetchTransactions(ctx.Request().Context(), filter) if err != nil { @@ -487,12 +538,9 @@ func (si *ServerImplementation) SearchForTransactions(ctx echo.Context, params g return badRequest(ctx, err.Error()) } - if filter.AddressRole != 0 { - var address basics.Address - copy(address[:], filter.Address) - if address.IsZero() { - return badRequest(ctx, errZeroAddressRole) - } + err = validateTransactionFilter(&filter) + if err != nil { + return badRequest(ctx, err.Error()) } // Fetch the transactions diff --git a/api/handlers_test.go b/api/handlers_test.go index cd53d4c3f..1de3da282 100644 --- a/api/handlers_test.go +++ b/api/handlers_test.go @@ -15,6 +15,7 @@ import ( "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/algorand/go-algorand-sdk/encoding/msgpack" "github.com/algorand/go-algorand/data/basics" @@ -136,22 +137,6 @@ func TestTransactionParamToTransactionFilter(t *testing.T) { }, nil, }, - { - name: "Round + Min/Max Error", - params: generated.SearchForTransactionsParams{ - Round: uint64Ptr(10), - MinRound: uint64Ptr(5), - MaxRound: uint64Ptr(15), - }, - filter: idb.TransactionFilter{}, - errorContains: []string{errInvalidRoundAndMinMax}, - }, - { - name: "Swapped Min/Max Round", - params: generated.SearchForTransactionsParams{MinRound: uint64Ptr(20), MaxRound: uint64Ptr(10)}, - filter: idb.TransactionFilter{}, - errorContains: []string{errInvalidRoundMinMax}, - }, { name: "Illegal Address", params: generated.SearchForTransactionsParams{Address: strPtr("Not-our-base32-thing")}, @@ -215,23 +200,90 @@ func TestTransactionParamToTransactionFilter(t *testing.T) { } for _, test := range tests { - //test := test t.Run(test.name, func(t *testing.T) { - //t.Parallel() filter, err := transactionParamsToTransactionFilter(test.params) - if test.errorContains != nil { + if len(test.errorContains) > 0 { + require.Error(t, err) for _, msg := range test.errorContains { assert.Contains(t, err.Error(), msg) } } else { assert.NoError(t, err) - assert.Equal(t, test.errorContains != nil, err != nil) assert.Equal(t, test.filter, filter) } }) } } +func TestValidateTransactionFilter(t *testing.T) { + tests := []struct { + name string + filter idb.TransactionFilter + errorContains []string + }{ + { + "Default", + idb.TransactionFilter{Limit: defaultTransactionsLimit}, + nil, + }, + { + name: "Round + MinRound Error", + filter: idb.TransactionFilter{ + Round: uint64Ptr(10), + MaxRound: 15, + }, + errorContains: []string{errInvalidRoundAndMinMax}, + }, + { + name: "Round + MinRound Error", + filter: idb.TransactionFilter{ + Round: uint64Ptr(10), + MinRound: 5, + }, + errorContains: []string{errInvalidRoundAndMinMax}, + }, + { + name: "Swapped Min/Max Round", + filter: idb.TransactionFilter{ + MinRound: 15, + MaxRound: 5, + }, + errorContains: []string{errInvalidRoundMinMax}, + }, + { + name: "Zero address close address role", + filter: idb.TransactionFilter{ + Address: addrSlice(basics.Address{}), + AddressRole: idb.AddressRoleSender | idb.AddressRoleCloseRemainderTo, + }, + errorContains: []string{errZeroAddressCloseRemainderToRole}, + }, + { + name: "Zero address asset sender and asset close address role", + filter: idb.TransactionFilter{ + Address: addrSlice(basics.Address{}), + AddressRole: idb.AddressRoleAssetSender | idb.AddressRoleAssetCloseTo, + }, + errorContains: []string{ + errZeroAddressAssetSenderRole, errZeroAddressAssetCloseToRole}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := validateTransactionFilter(&test.filter) + if len(test.errorContains) > 0 { + require.Error(t, err) + for _, msg := range test.errorContains { + assert.Contains(t, err.Error(), msg) + } + } else { + assert.NoError(t, err) + } + }) + } +} + func loadResourceFileOrPanic(path string) []byte { data, err := ioutil.ReadFile(path) if err != nil { diff --git a/api/pointer_utils.go b/api/pointer_utils.go index 0945d41a9..03d38c6e6 100644 --- a/api/pointer_utils.go +++ b/api/pointer_utils.go @@ -101,3 +101,9 @@ func strArrayPtr(x []string) *[]string { } return &x } + +func addrSlice(x basics.Address) []byte { + xx := new(basics.Address) + *xx = x; + return xx[:] +} From 5406b6c256c8825e000b36c33efe1cde696cbd63 Mon Sep 17 00:00:00 2001 From: Tolik Zinovyev Date: Mon, 8 Nov 2021 17:55:45 -0500 Subject: [PATCH 6/6] Run make fmt. --- api/error_messages.go | 46 +++++++++++++++++++++---------------------- api/handlers.go | 6 +++--- api/handlers_test.go | 16 +++++++-------- api/pointer_utils.go | 2 +- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/api/error_messages.go b/api/error_messages.go index e9610cf11..fe569f47c 100644 --- a/api/error_messages.go +++ b/api/error_messages.go @@ -8,30 +8,30 @@ import ( ) const ( - errInvalidRoundAndMinMax = "cannot specify round and min-round/max-round" - errInvalidRoundMinMax = "min-round must be less than max-round" - errUnableToParseAddress = "unable to parse address" - errInvalidCreatorAddress = "found an invalid creator address" - errUnableToParseBase64 = "unable to parse base64 data" - errUnableToParseDigest = "unable to parse base32 digest data" - errUnableToParseNext = "unable to parse next token" - errUnableToDecodeTransaction = "unable to decode transaction bytes" - errFailedSearchingAccount = "failed while searching for account" - errNoAccountsFound = "no accounts found for address" - errNoAssetsFound = "no assets found for asset-id" - errNoTransactionFound = "no transaction found for transaction id" - errMultipleTransactions = "multiple transactions found for this txid, please contact us this shouldn't happen" - errMultipleAccounts = "multiple accounts found for this address, please contact us this shouldn't happen" - errMultipleAssets = "multiple assets found for this id, please contact us this shouldn't happen" - errMultiAcctRewind = "multiple accounts rewind is not supported by this server" - errRewindingAccount = "error while rewinding account" - errLookingUpBlock = "error while looking up block for round" - errTransactionSearch = "error while searching for transaction" - errSpecialAccounts = "indexer doesn't support fee sink and rewards pool accounts, please refer to algod for relevant information" - errFailedLoadSpecialAccounts = "failed to retrieve special accounts" + errInvalidRoundAndMinMax = "cannot specify round and min-round/max-round" + errInvalidRoundMinMax = "min-round must be less than max-round" + errUnableToParseAddress = "unable to parse address" + errInvalidCreatorAddress = "found an invalid creator address" + errUnableToParseBase64 = "unable to parse base64 data" + errUnableToParseDigest = "unable to parse base32 digest data" + errUnableToParseNext = "unable to parse next token" + errUnableToDecodeTransaction = "unable to decode transaction bytes" + errFailedSearchingAccount = "failed while searching for account" + errNoAccountsFound = "no accounts found for address" + errNoAssetsFound = "no assets found for asset-id" + errNoTransactionFound = "no transaction found for transaction id" + errMultipleTransactions = "multiple transactions found for this txid, please contact us this shouldn't happen" + errMultipleAccounts = "multiple accounts found for this address, please contact us this shouldn't happen" + errMultipleAssets = "multiple assets found for this id, please contact us this shouldn't happen" + errMultiAcctRewind = "multiple accounts rewind is not supported by this server" + errRewindingAccount = "error while rewinding account" + errLookingUpBlock = "error while looking up block for round" + errTransactionSearch = "error while searching for transaction" + errSpecialAccounts = "indexer doesn't support fee sink and rewards pool accounts, please refer to algod for relevant information" + errFailedLoadSpecialAccounts = "failed to retrieve special accounts" errZeroAddressCloseRemainderToRole = "searching transactions by zero address with close address role is not supported" - errZeroAddressAssetSenderRole = "searching transactions by zero address with asset sender role is not supported" - errZeroAddressAssetCloseToRole = "searching transactions by zero address with asset close address role is not supported" + errZeroAddressAssetSenderRole = "searching transactions by zero address with asset sender role is not supported" + errZeroAddressAssetCloseToRole = "searching transactions by zero address with asset close address role is not supported" ) var errUnknownAddressRole string diff --git a/api/handlers.go b/api/handlers.go index 33d168da4..cf66e569a 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -74,13 +74,13 @@ func validateTransactionFilter(filter *idb.TransactionFilter) error { var address basics.Address copy(address[:], filter.Address) if address.IsZero() { - if filter.AddressRole & idb.AddressRoleCloseRemainderTo != 0 { + if filter.AddressRole&idb.AddressRoleCloseRemainderTo != 0 { errorArr = append(errorArr, errZeroAddressCloseRemainderToRole) } - if filter.AddressRole & idb.AddressRoleAssetSender != 0 { + if filter.AddressRole&idb.AddressRoleAssetSender != 0 { errorArr = append(errorArr, errZeroAddressAssetSenderRole) } - if filter.AddressRole & idb.AddressRoleAssetCloseTo != 0 { + if filter.AddressRole&idb.AddressRoleAssetCloseTo != 0 { errorArr = append(errorArr, errZeroAddressAssetCloseToRole) } } diff --git a/api/handlers_test.go b/api/handlers_test.go index 1de3da282..c39099077 100644 --- a/api/handlers_test.go +++ b/api/handlers_test.go @@ -228,23 +228,23 @@ func TestValidateTransactionFilter(t *testing.T) { }, { name: "Round + MinRound Error", - filter: idb.TransactionFilter{ - Round: uint64Ptr(10), + filter: idb.TransactionFilter{ + Round: uint64Ptr(10), MaxRound: 15, }, errorContains: []string{errInvalidRoundAndMinMax}, }, { name: "Round + MinRound Error", - filter: idb.TransactionFilter{ - Round: uint64Ptr(10), + filter: idb.TransactionFilter{ + Round: uint64Ptr(10), MinRound: 5, }, errorContains: []string{errInvalidRoundAndMinMax}, }, { - name: "Swapped Min/Max Round", - filter: idb.TransactionFilter{ + name: "Swapped Min/Max Round", + filter: idb.TransactionFilter{ MinRound: 15, MaxRound: 5, }, @@ -253,7 +253,7 @@ func TestValidateTransactionFilter(t *testing.T) { { name: "Zero address close address role", filter: idb.TransactionFilter{ - Address: addrSlice(basics.Address{}), + Address: addrSlice(basics.Address{}), AddressRole: idb.AddressRoleSender | idb.AddressRoleCloseRemainderTo, }, errorContains: []string{errZeroAddressCloseRemainderToRole}, @@ -261,7 +261,7 @@ func TestValidateTransactionFilter(t *testing.T) { { name: "Zero address asset sender and asset close address role", filter: idb.TransactionFilter{ - Address: addrSlice(basics.Address{}), + Address: addrSlice(basics.Address{}), AddressRole: idb.AddressRoleAssetSender | idb.AddressRoleAssetCloseTo, }, errorContains: []string{ diff --git a/api/pointer_utils.go b/api/pointer_utils.go index 03d38c6e6..0aa08f9ee 100644 --- a/api/pointer_utils.go +++ b/api/pointer_utils.go @@ -104,6 +104,6 @@ func strArrayPtr(x []string) *[]string { func addrSlice(x basics.Address) []byte { xx := new(basics.Address) - *xx = x; + *xx = x return xx[:] }