Skip to content

Commit

Permalink
added validation for all fee operations to have negative value
Browse files Browse the repository at this point in the history
  • Loading branch information
raghavapamula committed Apr 12, 2022
2 parents de67541 + dcbcf2d commit 75dad3c
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 12 deletions.
15 changes: 14 additions & 1 deletion asserter/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,20 @@ func (a *Asserter) Operations( // nolint:gocognit
)
}

val, _ := new(big.Int).SetString(op.Amount.Value, 10)
val, err := types.BigInt(op.Amount.Value)
if err != nil {
return err
}

// Validate that fee operation amount is negative
if val.Sign() != -1 {
return fmt.Errorf(
"%w: operation index %d",
ErrFeeAmountNotNegative,
op.OperationIdentifier.Index,
)
}

feeTotal.Add(feeTotal, val)
feeCount++
}
Expand Down
90 changes: 90 additions & 0 deletions asserter/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,14 @@ func TestOperationsValidations(t *testing.T) {
},
}

invalidFeeAmount = &types.Amount{
Value: "100",
Currency: &types.Currency{
Symbol: "BTC",
Decimals: 8,
},
}

validAccount = &types.AccountIdentifier{
Address: "test",
}
Expand Down Expand Up @@ -513,6 +521,88 @@ func TestOperationsValidations(t *testing.T) {
construction: false,
err: ErrRelatedOperationInFeeNotAllowed,
},

"fee amount is non-negative": {
operations: []*types.Operation{
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(0),
},
Type: "PAYMENT",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: validDepositAmount,
},
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(1),
},
Type: "PAYMENT",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: &types.Amount{
Value: "-2000",
Currency: &types.Currency{
Symbol: "BTC",
Decimals: 8,
},
},
},
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(2),
},
Type: "FEE",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: invalidFeeAmount,
},
},
validationFilePath: "data/validation_fee_and_payment_unbalanced.json",
construction: false,
err: ErrFeeAmountNotNegative,
},

"fee amount is negative as expected": {
operations: []*types.Operation{
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(0),
},
Type: "PAYMENT",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: validDepositAmount,
},
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(1),
},
Type: "PAYMENT",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: &types.Amount{
Value: "-2000",
Currency: &types.Currency{
Symbol: "BTC",
Decimals: 8,
},
},
},
{
OperationIdentifier: &types.OperationIdentifier{
Index: int64(2),
},
Type: "FEE",
Status: types.String("SUCCESS"),
Account: validAccount,
Amount: validFeeAmount,
},
},
validationFilePath: "data/validation_fee_and_payment_unbalanced.json",
construction: false,
err: nil,
},
}

for name, test := range tests {
Expand Down
2 changes: 2 additions & 0 deletions asserter/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ var (
ErrFeeAmountNotBalancing = errors.New("fee amount doesn't balance")
ErrPaymentCountMismatch = errors.New("payment count doesn't match")
ErrFeeCountMismatch = errors.New("fee count doesn't match")
ErrFeeAmountNotNegative = errors.New("fee amount is not negative")

BlockErrs = []error{
ErrAmountValueMissing,
Expand Down Expand Up @@ -142,6 +143,7 @@ var (
ErrDuplicateRelatedTransaction,
ErrPaymentAmountNotBalancing,
ErrFeeAmountNotBalancing,
ErrFeeAmountNotNegative,
}
)

Expand Down
43 changes: 43 additions & 0 deletions mocks/utils/fetcher_helper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 15 additions & 8 deletions storage/modules/coin_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,21 +501,28 @@ func (c *CoinStorage) GetLargestCoin(
// This is used when importing prefunded addresses.
func (c *CoinStorage) SetCoinsImported(
ctx context.Context,
accountBalances []*utils.AccountBalance,
accts []*types.AccountIdentifier,
acctCoinsResp []*utils.AccountCoinsResponse,
) error {
var accountCoins []*types.AccountCoin
for _, accountBalance := range accountBalances {
for _, coin := range accountBalance.Coins {
accountCoin := &types.AccountCoin{
Account: accountBalance.Account,
// Request array length should always equal response array length.
// But we still check it for sure.
if len(accts) != len(acctCoinsResp) {
return errors.ErrCoinImportFailed
}

var acctCoins []*types.AccountCoin
for i, resp := range acctCoinsResp {
for _, coin := range resp.Coins {
acctCoin := &types.AccountCoin{
Account: accts[i],
Coin: coin,
}

accountCoins = append(accountCoins, accountCoin)
acctCoins = append(acctCoins, acctCoin)
}
}

if err := c.AddCoins(ctx, accountCoins); err != nil {
if err := c.AddCoins(ctx, acctCoins); err != nil {
return fmt.Errorf("%w: %v", errors.ErrCoinImportFailed, err)
}

Expand Down
14 changes: 11 additions & 3 deletions storage/modules/coin_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,15 @@ var (
},
},
}

acctCoins = []*utils.AccountCoinsResponse{
{
Coins: accBalance1.Coins,
},
{
Coins: accBalance2.Coins,
},
}
)

func TestCoinStorage(t *testing.T) {
Expand Down Expand Up @@ -961,9 +970,8 @@ func TestCoinStorage(t *testing.T) {
})

t.Run("SetCoinsImported", func(t *testing.T) {
accBalances := []*utils.AccountBalance{accBalance1, accBalance2}

err := c.SetCoinsImported(ctx, accBalances)
accts := []*types.AccountIdentifier{accBalance1.Account, accBalance2.Account}
err := c.SetCoinsImported(ctx, accts, acctCoins)
assert.NoError(t, err)

mockHelper.On(
Expand Down
59 changes: 59 additions & 0 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ type FetcherHelper interface {
block *types.PartialBlockIdentifier,
currencies []*types.Currency,
) (*types.BlockIdentifier, []*types.Amount, map[string]interface{}, *fetcher.Error)

AccountCoinsRetry(
ctx context.Context,
network *types.NetworkIdentifier,
acct *types.AccountIdentifier,
includeMempool bool,
currencies []*types.Currency,
) (*types.BlockIdentifier, []*types.Coin, map[string]interface{}, *fetcher.Error)
}

type BlockStorageHelper interface {
Expand Down Expand Up @@ -491,6 +499,57 @@ func GetAccountBalances(
return accountBalances, nil
}

// -------------------------------------------------------------------------------
// ----------------- Helper struct for fetching account coins --------------------
// -------------------------------------------------------------------------------

// AccountCoinsRequest defines the required information to get an account's coins.
type AccountCoinsRequest struct {
Account *types.AccountIdentifier
Network *types.NetworkIdentifier
Currencies []*types.Currency
IncludeMempool bool
}

// AccountCoins defines an account's coins info at tip.
type AccountCoinsResponse struct {
Coins []*types.Coin
}

// GetAccountCoins calls /account/coins endpoint and returns an array of coins at tip.
func GetAccountCoins(
ctx context.Context,
fetcher FetcherHelper,
acctCoinsReqs []*AccountCoinsRequest,
) ([]*AccountCoinsResponse, error) {
var acctCoins []*AccountCoinsResponse
for _, req := range acctCoinsReqs {
_, coins, _, err := fetcher.AccountCoinsRetry(
ctx,
req.Network,
req.Account,
req.IncludeMempool,
req.Currencies,
)

if err != nil {
return nil, err.Err
}

resp := &AccountCoinsResponse{
Coins: coins,
}

acctCoins = append(acctCoins, resp)
}

return acctCoins, nil
}

// -------------------------------------------------------------------------------
// ------------------- End of helper struct for account coins --------------------
// -------------------------------------------------------------------------------

// AtTip returns a boolean indicating if a block timestamp
// is within tipDelay from the current time.
func AtTip(
Expand Down

0 comments on commit 75dad3c

Please sign in to comment.