node(governor): add some type safety; rename decimals to scalingFactor

- Change TokenConfigEntry.Decimals field from int64 to uint8 for more appropriate data type
- Rename tokenEntry.decimals to scalingFactor with clarifying comment explaining it represents 10^decimals
- Add comprehensive error checking in computeValue function to prevent nil pointer dereferences and division by zero
- Add test cases to validate token prices are positive and decimals are within sensible bounds (0-24)

Author: johnsaigle

node/pkg/governor/governor.go

@@ -59,7 +59,7 @@ type (
 		Addr        string
 		Symbol      string
 		CoinGeckoId string
-		Decimals    int64
+		Decimals    uint8
 		Price       float64
 	}
 
@@ -78,8 +78,9 @@ type (
 
 	// Payload of the map of the tokens being monitored
 	tokenEntry struct {
-		price          *big.Float
-		decimals       *big.Int
+		price *big.Float
+		// scalingFactor is 10 to the power of the number of decimals of the [TokenConfigEntry].
+		scalingFactor  *big.Int
 		symbol         string
 		coinGeckoId    string
 		token          tokenKey
@@ -329,8 +330,9 @@ func (gov *ChainGovernor) initConfig() error {
 			dec = 8
 		}
 
-		decimalsFloat := big.NewFloat(math.Pow(10.0, float64(dec)))
-		decimals, _ := decimalsFloat.Int(nil)
+		// Calculate the scaling factor based on the number of decimals.
+		scalingFactorFloat := big.NewFloat(math.Pow(10.0, float64(dec)))
+		scalingFactor, _ := scalingFactorFloat.Int(nil)
 
 		// Some Solana tokens don't have the symbol set. In that case, use the chain and token address as the symbol.
 		symbol := ct.Symbol
@@ -340,12 +342,12 @@ func (gov *ChainGovernor) initConfig() error {
 
 		key := tokenKey{chain: vaa.ChainID(ct.Chain), addr: addr}
 		te := &tokenEntry{
-			cfgPrice:    cfgPrice,
-			price:       initialPrice,
-			decimals:    decimals,
-			symbol:      symbol,
-			coinGeckoId: ct.CoinGeckoId,
-			token:       key,
+			cfgPrice:      cfgPrice,
+			price:         initialPrice,
+			scalingFactor: scalingFactor,
+			symbol:        symbol,
+			coinGeckoId:   ct.CoinGeckoId,
+			token:         key,
 		}
 		te.updatePrice()
 
@@ -366,8 +368,8 @@ func (gov *ChainGovernor) initConfig() error {
 				zap.String("symbol", te.symbol),
 				zap.String("coinGeckoId", te.coinGeckoId),
 				zap.String("price", te.price.String()),
-				zap.Int64("decimals", dec),
-				zap.Int64("origDecimals", ct.Decimals),
+				zap.Uint8("decimals", dec),
+				zap.Uint8("origDecimals", ct.Decimals),
 			)
 		}
 	}
@@ -946,15 +948,31 @@ func (gov *ChainGovernor) CheckPendingForTime(now time.Time) ([]*common.MessageP
 	return msgsToPublish, nil
 }
 
+// computeValue computes the USD value of a transfer.
 func computeValue(amount *big.Int, token *tokenEntry) (uint64, error) {
+
+	if token.scalingFactor == nil {
+		return 0, fmt.Errorf("scalingFactor is nil")
+	}
+
+	if token.price == nil {
+		return 0, fmt.Errorf("price is nil")
+	}
+
+	// Prevent division by zero panic.
+	// Shouldn't occur in practice as scalingFactor is always a power of 10.
+	if token.scalingFactor.Cmp(big.NewInt(0)) == 0 {
+		return 0, fmt.Errorf("scalingFactor is zero")
+	}
+
 	amountFloat := new(big.Float)
 	amountFloat = amountFloat.SetInt(amount)
 
 	valueFloat := new(big.Float)
 	valueFloat = valueFloat.Mul(amountFloat, token.price)
 
 	valueBigInt, _ := valueFloat.Int(nil)
-	valueBigInt = valueBigInt.Div(valueBigInt, token.decimals)
+	valueBigInt = valueBigInt.Div(valueBigInt, token.scalingFactor)
 
 	if !valueBigInt.IsUint64() {
 		return 0, fmt.Errorf("value is too large to fit in uint64")

node/pkg/governor/governor_test.go

@@ -43,7 +43,7 @@ func (gov *ChainGovernor) initConfigForTest(
 	decimals, _ := decimalsFloat.Int(nil)
 	key := tokenKey{chain: tokenChainID, addr: tokenAddr}
 
-	gov.tokens[key] = &tokenEntry{price: price, decimals: decimals, symbol: tokenSymbol, token: key}
+	gov.tokens[key] = &tokenEntry{price: price, scalingFactor: decimals, symbol: tokenSymbol, token: key}
 }
 
 func (gov *ChainGovernor) setDayLengthInMinutes(minimum int) {
@@ -89,17 +89,19 @@ func (gov *ChainGovernor) setTokenForTesting(
 	gov.mutex.Lock()
 	defer gov.mutex.Unlock()
 
+	const Decimals = 8
+
 	tokenAddr, err := vaa.StringToAddress(tokenAddrStr)
 	if err != nil {
 		return err
 	}
 
 	bigPrice := big.NewFloat(price)
-	decimalsFloat := big.NewFloat(math.Pow(10.0, float64(8)))
-	decimals, _ := decimalsFloat.Int(nil)
+	scalingFactorFloat := big.NewFloat(math.Pow(10.0, float64(Decimals)))
+	scalingFactor, _ := scalingFactorFloat.Int(nil)
 
 	key := tokenKey{chain: tokenChainID, addr: tokenAddr}
-	te := &tokenEntry{cfgPrice: bigPrice, price: bigPrice, decimals: decimals, symbol: symbol, coinGeckoId: symbol, token: key, flowCancels: flowCancels}
+	te := &tokenEntry{cfgPrice: bigPrice, price: bigPrice, scalingFactor: scalingFactor, symbol: symbol, coinGeckoId: symbol, token: key, flowCancels: flowCancels}
 	gov.tokens[key] = te
 	cge, cgExists := gov.tokensByCoinGeckoId[te.coinGeckoId]
 	if !cgExists {

node/pkg/governor/mainnet_tokens_test.go

@@ -2,6 +2,7 @@ package governor
 
 import (
 	"fmt"
+	"slices"
 	"strings"
 	"testing"
 
@@ -33,6 +34,26 @@ func TestTokenListAddressSize(t *testing.T) {
 	}
 }
 
+func TestTokenPositivePrices(t *testing.T) {
+	tokenConfigEntries := TokenList()
+
+	for tokenConfigEntry := range slices.Values(tokenConfigEntries) {
+		assert.Greater(t, tokenConfigEntry.Price, float64(0), "Token price must be greater than zero")
+	}
+}
+
+func TestTokenSensibleDecimals(t *testing.T) {
+	tokenConfigEntries := TokenList()
+	// This is the global maximum number of decimals among the tokens we have configured. (due to NEAR)
+	// A larger number may be fine but it's unusual, so it's worth flagging.
+	const maxDecimals = 24
+
+	for tokenConfigEntry := range slices.Values(tokenConfigEntries) {
+		assert.GreaterOrEqual(t, tokenConfigEntry.Decimals, uint8(0), "Token decimals must be greater than or equal to zero")
+		assert.LessOrEqual(t, tokenConfigEntry.Decimals, uint8(maxDecimals), fmt.Sprintf("Token decimals must be less than or equal to %d but got %d. details: %v", maxDecimals, tokenConfigEntry.Decimals, tokenConfigEntry))
+	}
+}
+
 // Flag a situation where a Governed chain does not have any governed assets. Often times when adding a mainnet chain,
 // a list of tokens will be added so that they can be governed. (These tokens are sourced by CoinGecko or manually
 // populated.) While this is not a hard requirement, it may represent that a developer has forgotten to take the step