Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ jobs:
- name: Formatting checks
run: ./scripts/lint.sh -l -g format
- name: Install linters
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.8.0
run: curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.12.2
Comment thread
johnsaigle marked this conversation as resolved.
- name: Run linters
run: make generate && golangci-lint --version && ./scripts/lint.sh -g lint
- name: Ensure generated proto matches
Expand Down
2 changes: 1 addition & 1 deletion node/cmd/ccq/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type (

User struct {
UserName string `json:"userName"`
APIKey string `json:"apiKey"`
APIKey string `json:"apiKey"` // #nosec G117 -- This is a config field name, not a hardcoded secret
AllowUnsigned bool `json:"allowUnsigned"`
AllowAnything bool `json:"allowAnything"`
RateLimit *float64 `json:"RateLimit"`
Expand Down
31 changes: 15 additions & 16 deletions node/cmd/guardiand/adminclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,13 @@ func runInjectGovernanceVAA(cmd *cobra.Command, args []string) {
}

func runFindMissingMessages(cmd *cobra.Command, args []string) {
chainID, err := strconv.Atoi(args[0])
// Effectively allow any uint16 value for ChainID here. A Guardian may want to find missing messages
// for deprecated chains, so we don't use [`vaa.StringToKnownChainID`] here.
//
// This value is later converted to uint32 for use in protobuf, but before that
// we should make sure it fits into uint16 to disallow numbers > max.Uint16
// from being serialized.
chainId, err := strconv.ParseUint(args[0], 10, 16)
if err != nil {
log.Fatalf("invalid chain ID: %v", err)
}
Expand All @@ -439,12 +445,8 @@ func runFindMissingMessages(cmd *cobra.Command, args []string) {
}
defer conn.Close()

if chainID > math.MaxUint16 {
log.Fatalf("chain ID is not a valid 16 bit unsigned integer: %v", err)
}

msg := nodev1.FindMissingMessagesRequest{
EmitterChain: uint32(chainID), // #nosec G115 -- This conversion is checked above
EmitterChain: uint32(chainId),
EmitterAddress: emitterAddress,
RpcBackfill: *shouldBackfill,
BackfillNodes: sdk.PublicRPCEndpoints,
Expand All @@ -470,13 +472,13 @@ func runDumpVAAByMessageID(cmd *cobra.Command, args []string) {
if len(parts) != 3 {
log.Fatalf("invalid message ID: %s", args[0])
}
chainID, err := strconv.ParseUint(parts[0], 10, 32)
// Parse string ChainID as uint16 to ensure it fits the vaa.ChainID type.
// There is no "known" check here as Guardians may want to dump messages
// even for deprecated chains.
chainId, err := strconv.ParseUint(parts[0], 10, 16)
if err != nil {
log.Fatalf("invalid chain ID: %v", err)
}
if chainID > math.MaxUint16 {
log.Fatalf("chain id must not exceed the max uint16: %v", chainID)
}
emitterAddress := parts[1]
seq, err := strconv.ParseUint(parts[2], 10, 64)
if err != nil {
Expand All @@ -494,7 +496,7 @@ func runDumpVAAByMessageID(cmd *cobra.Command, args []string) {

msg := publicrpcv1.GetSignedVAARequest{
MessageId: &publicrpcv1.MessageID{
EmitterChain: publicrpcv1.ChainID(chainID),
EmitterChain: publicrpcv1.ChainID(chainId),
EmitterAddress: emitterAddress,
Sequence: seq,
},
Expand Down Expand Up @@ -1083,15 +1085,11 @@ func runChainGovernorResetReleaseTimer(cmd *cobra.Command, args []string) {
}

func runPurgePythNetVaas(cmd *cobra.Command, args []string) {
daysOld, err := strconv.Atoi(args[0])
daysOld, err := strconv.ParseUint(args[0], 10, 64)
if err != nil {
log.Fatalf("invalid DAYS_OLD: %v", err)
}

if daysOld < 0 {
log.Fatalf("DAYS_OLD may not be negative")
}

logOnly := false
if len(args) > 1 {
if args[1] != "logonly" {
Expand Down Expand Up @@ -1237,6 +1235,7 @@ func runSignExistingVaasFromCSV(cmd *cobra.Command, args []string) {
}
resp, err := c.SignExistingVAA(ctx, &msg)
if err != nil {
// #nosec G706 -- row[0] is from admin CSV input, err is safe to log
log.Printf("signing VAA (%s)[%d] failed - skipping: %v", row[0], i, err)
continue
}
Expand Down
17 changes: 10 additions & 7 deletions node/cmd/guardiand/adminnodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"math"
"os"
"sort"
"strings"
Expand Down Expand Up @@ -179,15 +178,19 @@ func runListNodes(cmd *cobra.Command, args []string) {
truncAddrs := make(map[vaa.ChainID]string)
errors := map[vaa.ChainID]uint64{}
for _, n := range h.RawHeartbeat.Networks {
if n.Id > math.MaxUint16 {
log.Fatalf("heartbeat chain id is greater than MaxUint16: %v", n.Id)
// Convert uint32 to ChainID. We deliberately don't use [vaa.KnownChainIDFromNumber] here
// in case the Guardians are running with different versions and so have a different set of
// chain IDs that are considered "known".
chainId, err := vaa.ChainIDFromNumber[uint32](n.Id)
if err != nil {
log.Fatalf("invalid chain id in heartbeat: %v", err)
Comment thread
johnsaigle marked this conversation as resolved.
}
heights[vaa.ChainID(n.Id)] = n.Height
errors[vaa.ChainID(n.Id)] = n.ErrorCount
heights[chainId] = n.Height
errors[chainId] = n.ErrorCount
if len(n.ContractAddress) >= 16 {
truncAddrs[vaa.ChainID(n.Id)] = n.ContractAddress[:16]
truncAddrs[chainId] = n.ContractAddress[:16]
} else {
truncAddrs[vaa.ChainID(n.Id)] = "INVALID"
truncAddrs[chainId] = "INVALID"
}
}

Expand Down
8 changes: 6 additions & 2 deletions node/cmd/guardiand/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,8 +794,12 @@ func runNode(cmd *cobra.Command, args []string) {
}

// Node's main lifecycle context.
rootCtx, rootCtxCancel = context.WithCancel(context.Background())
defer rootCtxCancel()
// Keep the cancel func in a local var so linters can see
// that the WithCancel return value is actually invoked.
var cancel context.CancelFunc
rootCtx, cancel = context.WithCancel(context.Background())
rootCtxCancel = cancel
defer cancel()

// Create the Guardian Signer
guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(rootCtx, *guardianSignerUri, env == common.UnsafeDevNet)
Expand Down
14 changes: 9 additions & 5 deletions node/cmd/spy/spy.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (s *spyServer) PublishSignedVAA(vaaBytes []byte) error {
return err
}
}
sub.ch <- message{vaaBytes: vaaBytes} //nolint:channelcheck // Don't want to drop incoming VAAs
sub.ch <- message{vaaBytes: vaaBytes} // Note on channel capacity: Don't want to drop incoming VAAs
continue
}

Expand All @@ -146,7 +146,7 @@ func (s *spyServer) PublishSignedVAA(vaaBytes []byte) error {
return err
}
}
sub.ch <- message{vaaBytes: vaaBytes} //nolint:channelcheck // Don't want to drop incoming VAAs
sub.ch <- message{vaaBytes: vaaBytes} // Note on channel capacity: Don't want to drop incoming VAAs
}
}

Expand Down Expand Up @@ -252,7 +252,7 @@ func newSpyServer(logger *zap.Logger) *spyServer {
func DoWithTimeout(f func() error, d time.Duration) error {
errChan := make(chan error, 1)
go func() {
errChan <- f() //nolint:channelcheck // Has timeout below
errChan <- f() // Note on channel capacity: Has timeout below
close(errChan)
}()
t := time.NewTimer(d)
Expand Down Expand Up @@ -341,8 +341,12 @@ func runSpy(cmd *cobra.Command, args []string) {
}

// Node's main lifecycle context.
rootCtx, rootCtxCancel = context.WithCancel(context.Background())
defer rootCtxCancel()
// Keep the cancel func in a local var so linters can see
// that the WithCancel return value is actually invoked.
var cancel context.CancelFunc
rootCtx, cancel = context.WithCancel(context.Background())
rootCtxCancel = cancel
defer cancel()

// Inbound signed VAAs
signedInC := make(chan *gossipv1.SignedVAAWithQuorum, 1024)
Expand Down
1 change: 1 addition & 0 deletions node/cmd/txverifier/sui.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ func pullDigestsFromWormholeScan(ctx context.Context, logger *zap.Logger) ([]str
}

req.Header.Set("Accept", "application/json")
// #nosec G704 -- Hardcoded WormholeScan API URL
resp, err := http.DefaultClient.Do(req)

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions node/hack/evm_test/wstest.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func main() {
case <-ctx.Done():
return
case err := <-headerSubscription.Err():
errC <- fmt.Errorf("block subscription failed: %w", err) //nolint:channelcheck // The watcher will exit anyway
errC <- fmt.Errorf("block subscription failed: %w", err) // Note on channel capacity: The watcher will exit anyway
Comment thread
johnsaigle marked this conversation as resolved.
return
case block := <-headSink:
// These two pointers should have been checked before the event was placed on the channel, but just being safe.
Expand Down Expand Up @@ -114,7 +114,7 @@ func main() {
case <-ctx.Done():
return
case err := <-messageSub.Err():
errC <- fmt.Errorf("message subscription failed: %w", err) //nolint:channelcheck // The watcher will exit anyway
errC <- fmt.Errorf("message subscription failed: %w", err) // Note on channel capacity: The watcher will exit anyway
return
case ev := <-messageC:
logger.Info("Received a log event from the contract", zap.Any("ev", ev))
Expand Down
1 change: 1 addition & 0 deletions node/hack/release_verification/guardian_vaa_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func getValidatorIndexForChain(chainId vaa.ChainID, onlyafter time.Time) (map[ui
}
req.Header.Add("Accept", "*/*")

// #nosec G704 -- Maintenance script calling hardcoded WormholeScan API
res, err := client.Do(req)
if err != nil {
return nil, err
Expand Down
4 changes: 4 additions & 0 deletions node/hack/repair_eth/repair_eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ func getCurrentHeight(chainId vaa.ChainID, ctx context.Context, c *http.Client,
}
req = addUserAgent(req)

// #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API
resp, err := c.Do(req.WithContext(ctx))
if err != nil {
return 0, fmt.Errorf("failed to get current height: %w", err)
Expand Down Expand Up @@ -184,6 +185,7 @@ func getLogs(chainId vaa.ChainID, ctx context.Context, c *http.Client, api, key,
}
req = addUserAgent(req)

// #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API
resp, err := c.Do(req)
if err != nil {
return nil, fmt.Errorf("failed to get logs: %w", err)
Expand Down Expand Up @@ -487,13 +489,15 @@ func main() {
panic(err)
}
req = addUserAgent(req)
// #nosec G704 -- Maintenance script calling hardcoded or config-provided explorer API
resp, err := c.Do(req)
if err != nil {
log.Fatalf("verify: %v", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
// #nosec G706 -- Logging response status code from trusted API
log.Printf("status %d, retrying", resp.StatusCode)
time.Sleep(5 * time.Second)
continue
Expand Down
2 changes: 2 additions & 0 deletions node/hack/repair_solana/repair.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ func main() {
if err != nil {
panic(err)
}
// #nosec G704 -- Maintenance script calling public Wormhole RPC endpoint
Comment thread
johnsaigle marked this conversation as resolved.
resp, err := hc.Do(req)
if err != nil {
log.Fatalf("verify: %v", err)
Expand All @@ -228,6 +229,7 @@ func main() {
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
// #nosec G706 -- Logging response status code from trusted API
log.Printf("status %d, retrying", resp.StatusCode)
time.Sleep(5 * time.Second)
continue
Expand Down
7 changes: 4 additions & 3 deletions node/pkg/adminrpc/adminserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ func (s *nodePrivilegedService) InjectGovernanceVAA(ctx context.Context, req *no

vaaInjectionsTotal.Inc()

s.injectC <- &common.MessagePublication{ //nolint:channelcheck // Only blocks this command
s.injectC <- &common.MessagePublication{ // Note on channel capacity: Only blocks this command
TxID: ethcommon.Hash{}.Bytes(),
Timestamp: v.Timestamp,
Nonce: v.Nonce,
Expand Down Expand Up @@ -1102,6 +1102,7 @@ func (s *nodePrivilegedService) fetchMissing(
return false, fmt.Errorf("failed to create request: %w", err)
}

// #nosec G704 -- Admin RPC: BackfillNodes from authorized admin request
resp, err := c.Do(req)
if err != nil {
s.logger.Warn("failed to fetch missing VAA",
Expand Down Expand Up @@ -1159,7 +1160,7 @@ func (s *nodePrivilegedService) fetchMissing(
// Inject into the gossip signed VAA receive path.
// This has the same effect as if the VAA was received from the network
// (verifying signature, storing in local DB...).
s.signedInC <- &gossipv1.SignedVAAWithQuorum{ //nolint:channelcheck // Only blocks this command
s.signedInC <- &gossipv1.SignedVAAWithQuorum{ // Note on channel capacity: Only blocks this command
Vaa: vaaBytes,
}

Expand Down Expand Up @@ -1678,7 +1679,7 @@ func (s *nodePrivilegedService) GetAndObserveMissingVAAs(ctx context.Context, re
Timeout: 30 * time.Second,
}

// Call the cloud function to get the missing VAAs
// #nosec G704 -- Admin RPC: URL from authorized admin request
results, err := client.Do(httpRequest)
if err != nil {
fmt.Printf("GetAndObserveMissingVAAs: error making http request: %s\n", err)
Expand Down
1 change: 1 addition & 0 deletions node/pkg/altpub/alternate_pub.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ func (ap *AlternatePublisher) httpPost(ctx context.Context, client *http.Client,
r.Header.Add("Content-Type", "application/octet-stream")

start := time.Now()
// #nosec G704 -- URL configured at startup, not user input
resp, err := client.Do(r)
if err != nil {
if strings.Contains(err.Error(), "connection refused") {
Expand Down
2 changes: 2 additions & 0 deletions node/pkg/common/nodekey.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
)

func GetOrCreateNodeKey(logger *zap.Logger, path string) (crypto.PrivKey, error) {
// #nosec G703 -- Path comes from operator configuration, not user input
b, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
Expand All @@ -25,6 +26,7 @@ func GetOrCreateNodeKey(logger *zap.Logger, path string) (crypto.PrivKey, error)
panic(marshalErr)
}

// #nosec G703 -- Path comes from operator configuration, not user input
err = os.WriteFile(path, s, 0600)
if err != nil {
return nil, fmt.Errorf("failed to write node key: %w", err)
Expand Down
8 changes: 5 additions & 3 deletions node/pkg/common/symmetric.go
Comment thread
johnsaigle marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ func EncryptAESGCM(plaintext, key []byte) ([]byte, error) {
if err != nil {
return nil, fmt.Errorf("failed to create gcm: %v", err)
}
nonce := make([]byte, gcm.NonceSize())
// Preallocate the output to avoid an extra allocation in append(nonce, out...).
out := make([]byte, gcm.NonceSize(), gcm.NonceSize()+len(plaintext)+gcm.Overhead())
nonce := out[:gcm.NonceSize()]
if _, err = rand.Read(nonce); err != nil {
return nil, fmt.Errorf("failed to read random data: %v", err)
}
out := gcm.Seal(nil, nonce, plaintext, nil)
return append(nonce, out...), nil
out = gcm.Seal(out, nonce, plaintext, nil)
return out, nil
}
14 changes: 9 additions & 5 deletions node/pkg/governor/governor_monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func (gov *ChainGovernor) GetAvailableNotionalByChain() (resp []*publicrpcv1.Gov
zap.Uint64("dailyLimit", ce.dailyLimit),
zap.Int64("netUsage", netUsage),
zap.Error(err))
} else if uint64(netUsage) > ce.dailyLimit {
} else if uint64(netUsage) > ce.dailyLimit { // #nosec G115 -- netUsage is checked to be non-negative above
gov.logger.Warn("GetAvailableNotionalByChain: net value for chain exceeds daily limit even though flow cancel is disabled",
zap.String("chainID", chainId.String()),
zap.Uint64("dailyLimit", ce.dailyLimit),
Expand Down Expand Up @@ -515,13 +515,17 @@ func (gov *ChainGovernor) CollectMetrics(ctx context.Context, hb *gossipv1.Heart
continue
}

if n.Id > math.MaxUint16 {
gov.logger.Error("CollectMetrics: chain id is not a valid uint16", zap.Uint32("chain_id", n.Id))
// The Governor is only concerned with registered chains. However, it may be possible that Guardians
// are operating with different versions, or may have pending messages from the last 24h for a chain
// that was just deprecated.
// In any case, any uint16 value is accepted here as the worst-case scenario is that the Guardian's
// heartbeat will contain data about a chain ID being unset here.
chain, err := vaa.ChainIDFromNumber[uint32](n.Id)
if err != nil {
gov.logger.Error("CollectMetrics: chain id is not valid", zap.Uint32("chain_id", n.Id), zap.Error(err))
continue
}

chain := vaa.ChainID(n.Id)

chainId := fmt.Sprint(n.Id)
enabled := "0"
totalNotional := "0"
Expand Down
2 changes: 1 addition & 1 deletion node/pkg/governor/governor_prices.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (gov *ChainGovernor) queryCoinGecko(ctx context.Context) error {
for {
select {
case <-ticker.C:
throttle <- 1 //nolint:channelcheck // We want this to block for throttling
throttle <- 1 // Note on channel capacity: We want this to block for throttling
case <-ctx.Done():
return
}
Expand Down
Loading
Loading