Skip to content

Commit

Permalink
chore: gas price validation cleanup from peer review
Browse files Browse the repository at this point in the history
Co-authored-by: Dan Kanefsky <dan@noble.xyz>
  • Loading branch information
johnletey and boojamya committed Oct 24, 2024
1 parent de5febf commit dac14ba
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
13 changes: 8 additions & 5 deletions keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,16 @@ func TestUpdateGasPrices(t *testing.T) {
// ASSERT: The action should've failed due to repeated denom.
require.ErrorContains(t, err, "denom uusdc is repeated")

// ACT: Attempt to update gas prices. Len(GasPrices) > 1. Unsorted denoms.
// ACT: Attempt to update gas prices. Len(GasPrices) > 1. Invalid second denom.
_, err = server.UpdateGasPrices(ctx, &types.MsgUpdateGasPrices{
Signer: "authority",
GasPrices: sdk.DecCoins{USDC, EURe},
Signer: "authority",
GasPrices: sdk.DecCoins{
EURe,
sdk.DecCoin{Denom: "-", Amount: usdAmount.MulInt64(-1)},
},
})
// ASSERT: The action should've failed due to unsorted denoms.
require.ErrorContains(t, err, "denom ueure is not sorted")
// ASSERT: The action should've failed due to invalid denom.
require.ErrorContains(t, err, "invalid denom: -")

// ACT: Attempt to update gas prices. Len(GasPrices) > 1. Invalid second amount.
_, err = server.UpdateGasPrices(ctx, &types.MsgUpdateGasPrices{
Expand Down
13 changes: 2 additions & 11 deletions types/globalfee.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,30 @@ import (
)

// Validate is a utility that verifies the provided gas prices. It ensures that
// entries are sorted, don't contain negative amounts, and don't contain invalid
// or repeated denoms.
// prices are not negative, and that denoms are both valid and not repeated.
func (raw *GasPrices) Validate() error {
gasPrices := raw.Value

switch gasPrices.Len() {
case 0:
return nil
case 1:
return ValidateGasPrice(gasPrices[0])
default:
if err := ValidateGasPrice(gasPrices[0]); err != nil {
return err
}

denom := gasPrices[0].Denom
seen := make(map[string]bool)
seen[denom] = true
seen[gasPrices[0].Denom] = true

for _, gasPrice := range gasPrices[1:] {
if seen[gasPrice.Denom] {
return fmt.Errorf("denom %s is repeated", gasPrice.Denom)
}

if gasPrice.Denom <= denom {
return fmt.Errorf("denom %s is not sorted", gasPrice.Denom)
}

if err := ValidateGasPrice(gasPrice); err != nil {
return err
}

denom = gasPrice.Denom
seen[gasPrice.Denom] = true
}

Expand Down

0 comments on commit dac14ba

Please sign in to comment.