Addressed TODOs

Author: amit-momin

pkg/solana/ccip/chainaccessor/config.go

@@ -20,57 +20,90 @@ import (
 // https://github.com/smartcontractkit/chainlink-ccip/blob/7cae1b8434dd376eb70f2ddaace43093982f3a57/chains/solana/contracts/programs/rmn-remote/src/state.rs#L20-L27
 var globalCurseValue = []byte{1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}
 
-// getOffRampStaticConfig retrieves static configuration for the off-ramp contract
-func (a *SolanaAccessor) getOffRampStaticConfig(ctx context.Context) (ccipocr3.OffRampStaticChainConfig, error) {
+// getOffRampConfig retrieves static, dynamic, commitOCR3, and execOCR3 configurations for the off-ramp contract
+func (a *SolanaAccessor) getOffRampConfig(ctx context.Context) (ccipocr3.OfframpConfig, error) {
 	offrampAddr, err := a.getBinding(consts.ContractNameOffRamp)
 	if err != nil {
-		return ccipocr3.OffRampStaticChainConfig{}, fmt.Errorf("failed to get binding for offramp: %w", err)
+		return ccipocr3.OfframpConfig{}, fmt.Errorf("failed to get binding for offramp: %w", err)
 	}
 
 	config, err := a.getOfframpConfig(ctx, offrampAddr)
 	if err != nil {
-		return ccipocr3.OffRampStaticChainConfig{}, err
+		return ccipocr3.OfframpConfig{}, err
 	}
 
 	offrampRefAddress, err := a.getOfframpReferenceAddresses(ctx, offrampAddr)
 	if err != nil {
-		return ccipocr3.OffRampStaticChainConfig{}, err
+		return ccipocr3.OfframpConfig{}, err
 	}
 
-	return ccipocr3.OffRampStaticChainConfig{
+	staticConfig := ccipocr3.OffRampStaticChainConfig{
 		ChainSelector:        ccipocr3.ChainSelector(config.SvmChainSelector),
 		GasForCallExactCheck: 0,
 		RmnRemote:            offrampRefAddress.RmnRemote.Bytes(),
 		TokenAdminRegistry:   []byte{}, // PDA dependent on the token address
 		NonceManager:         offrampRefAddress.Router.Bytes(),
-	}, nil
-}
+	}
 
-// getOffRampDynamicConfig retrieves dynamic configuration for the off-ramp contract
-func (a *SolanaAccessor) getOffRampDynamicConfig(ctx context.Context) (ccipocr3.OffRampDynamicChainConfig, error) {
-	offrampAddr, err := a.getBinding(consts.ContractNameOffRamp)
-	if err != nil {
-		return ccipocr3.OffRampDynamicChainConfig{}, fmt.Errorf("failed to get binding for offramp: %w", err)
+	dynamicConfig := ccipocr3.OffRampDynamicChainConfig{
+		FeeQuoter:                               offrampRefAddress.FeeQuoter.Bytes(),
+		PermissionLessExecutionThresholdSeconds: uint32(config.EnableManualExecutionAfter), // nolint:gosec // G115: value validated to be within uint32 max above
+		IsRMNVerificationDisabled:               true,
+		MessageInterceptor:                      []byte{}, // expected to be empty for solana
 	}
 
-	config, err := a.getOfframpConfig(ctx, offrampAddr)
-	if err != nil {
-		return ccipocr3.OffRampDynamicChainConfig{}, err
+	commitConfig := config.Ocr3[0]
+	commitOCR3Config := ccipocr3.OCRConfigResponse{
+		OCRConfig: ccipocr3.OCRConfig{
+			ConfigInfo: ccipocr3.ConfigInfo{
+				ConfigDigest:                   commitConfig.ConfigInfo.ConfigDigest,
+				F:                              commitConfig.ConfigInfo.F,
+				N:                              commitConfig.ConfigInfo.N,
+				IsSignatureVerificationEnabled: commitConfig.ConfigInfo.IsSignatureVerificationEnabled == 1,
+			},
+			Signers:      convertSignersType(commitConfig.Signers),
+			Transmitters: convertTransmittersType(commitConfig.Transmitters),
+		},
 	}
 
-	offrampRefAddress, err := a.getOfframpReferenceAddresses(ctx, offrampAddr)
-	if err != nil {
-		return ccipocr3.OffRampDynamicChainConfig{}, err
+	executeConfig := config.Ocr3[1]
+	executeOCR3Config := ccipocr3.OCRConfigResponse{
+		OCRConfig: ccipocr3.OCRConfig{
+			ConfigInfo: ccipocr3.ConfigInfo{
+				ConfigDigest:                   executeConfig.ConfigInfo.ConfigDigest,
+				F:                              executeConfig.ConfigInfo.F,
+				N:                              executeConfig.ConfigInfo.N,
+				IsSignatureVerificationEnabled: executeConfig.ConfigInfo.IsSignatureVerificationEnabled == 1,
+			},
+			Signers:      convertSignersType(executeConfig.Signers),
+			Transmitters: convertTransmittersType(executeConfig.Transmitters),
+		},
 	}
 
-	return ccipocr3.OffRampDynamicChainConfig{
-		FeeQuoter:                               offrampRefAddress.FeeQuoter.Bytes(),
-		PermissionLessExecutionThresholdSeconds: uint32(config.EnableManualExecutionAfter), // TODO: is this conversion dangerous?
-		IsRMNVerificationDisabled:               true,
-		MessageInterceptor:                      []byte{},
+	return ccipocr3.OfframpConfig{
+		CommitLatestOCRConfig: commitOCR3Config,
+		ExecLatestOCRConfig:   executeOCR3Config,
+		StaticConfig:          staticConfig,
+		DynamicConfig:         dynamicConfig,
 	}, nil
 }
 
+func convertSignersType(signers [16][20]uint8) [][]byte {
+	newSigners := make([][]byte, 0, len(signers))
+	for _, signer := range signers {
+		newSigners = append(newSigners, signer[:])
+	}
+	return newSigners
+}
+
+func convertTransmittersType(transmitters [16][32]uint8) [][]byte {
+	newTransmitters := make([][]byte, 0, len(transmitters))
+	for _, transmitter := range transmitters {
+		newTransmitters = append(newTransmitters, transmitter[:])
+	}
+	return newTransmitters
+}
+
 // getOffRampSourceChainConfigs retrieves source chain configurations from the off-ramp contract
 func (a *SolanaAccessor) getOffRampSourceChainConfigs(ctx context.Context, sourceChainSelectors []ccipocr3.ChainSelector) (map[ccipocr3.ChainSelector]ccipocr3.SourceChainConfig, error) {
 	offrampAddr, err := a.getBinding(consts.ContractNameOffRamp)

pkg/solana/ccip/chainaccessor/solana_accessor.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"math/big"
 	"sync"
 	"time"
@@ -62,9 +63,12 @@ func NewSolanaAccessor(
 	fee fees.Estimator,
 	addrCodec ccipocr3.ChainSpecificAddressCodec,
 ) (*SolanaAccessor, error) {
-	// TODO: validate state of client and logPoller (should be initialized in NewChain)
 	lggr := logger.Named(l, "SolanaAccessor")
 
+	if err := logPoller.Ready(); err != nil {
+		return nil, fmt.Errorf("log poller not ready: %w", err)
+	}
+
 	codec, err := buildEventsCodec()
 	if err != nil {
 		return nil, fmt.Errorf("failed to build codec parsed types: %w", err)
@@ -105,22 +109,11 @@ func (a *SolanaAccessor) GetAllConfigsLegacy(ctx context.Context, destChainSelec
 		// we're fetching config on the destination chain (offramp + fee quoter static config + RMN)
 
 		// OffRamp
-		offrampStaticConfig, err := a.getOffRampStaticConfig(ctx)
+		offrampConfig, err := a.getOffRampConfig(ctx)
 		if !errors.Is(err, ErrNoBindings) && err != nil {
 			return ccipocr3.ChainConfigSnapshot{}, nil, fmt.Errorf("failed to get current offramp static config: %w", err)
 		}
-		// TODO: assert offrampStaticConfig.ChainSelector == destChainSelector as a quick sanity check
-		offrampDynamicConfig, err := a.getOffRampDynamicConfig(ctx)
-		if !errors.Is(err, ErrNoBindings) && err != nil {
-			return ccipocr3.ChainConfigSnapshot{}, nil, fmt.Errorf("failed to get current offramp dynamic config: %w", err)
-		}
-		config.Offramp = ccipocr3.OfframpConfig{
-			// TODO: read OCR config from contract
-			CommitLatestOCRConfig: ccipocr3.OCRConfigResponse{},
-			ExecLatestOCRConfig:   ccipocr3.OCRConfigResponse{},
-			StaticConfig:          offrampStaticConfig,
-			DynamicConfig:         offrampDynamicConfig,
-		}
+		config.Offramp = offrampConfig
 
 		// FeeQuoter
 		feeQuoterStaticConfig, err := a.getFeeQuoterStaticConfig(ctx)
@@ -131,11 +124,17 @@ func (a *SolanaAccessor) GetAllConfigsLegacy(ctx context.Context, destChainSelec
 			StaticConfig: feeQuoterStaticConfig,
 		}
 
+		rmnRemoteProxyAddr, err := a.getBinding(consts.ContractNameRMNProxy)
+		if !errors.Is(err, ErrNoBindings) && err != nil {
+			return ccipocr3.ChainConfigSnapshot{}, nil, fmt.Errorf("failed to get binding for rmn remote proxy: %w", err)
+		}
+
 		// RMN
 		// TODO: RMNProxy should be an implementation detail hidden behind chainAccessor
 		config.RMNProxy = ccipocr3.RMNProxyConfig{
 			// TODO: point at a rmnremote address/router/offramp to allow fetching curseinfo
-			RemoteAddress: nil,
+			// There is no proxy for Solana so is it right to just set the "proxy" address as the remote address here?
+			RemoteAddress: rmnRemoteProxyAddr.Bytes(),
 		}
 		config.RMNRemote = ccipocr3.RMNRemoteConfig{
 			// We don't support RMN so return an empty config
@@ -382,10 +381,13 @@ func (a *SolanaAccessor) GetTokenPriceUSD(ctx context.Context, rawTokenAddress c
 		return ccipocr3.TimestampedUnixBig{}, fmt.Errorf("failed to get fee quoter billing token config account: %w", err)
 	}
 	value := new(big.Int).SetBytes(billingTokenConfig.UsdPerToken.Value[:])
+	if billingTokenConfig.UsdPerToken.Timestamp > math.MaxUint32 {
+		return ccipocr3.TimestampedUnixBig{}, fmt.Errorf("billing token config timestamp exceeds uint32 max: %d", billingTokenConfig.UsdPerToken.Timestamp)
+	}
 	return ccipocr3.TimestampedUnixBig{
 		Value: value,
 		// TODO: u64 -> u32? should we fix the onchain type?
-		Timestamp: uint32(billingTokenConfig.UsdPerToken.Timestamp), //nolint:gosec // G115
+		Timestamp: uint32(billingTokenConfig.UsdPerToken.Timestamp), //nolint:gosec // G115: validated to be within uint32 max above
 	}, nil
 }
 
@@ -447,7 +449,7 @@ func (a *SolanaAccessor) CommitReportsGTETimestamp(ctx context.Context, ts time.
 	limitSort := query.LimitAndSort{
 		SortBy: []query.SortBy{query.NewSortBySequence(query.Asc)},
 		Limit: query.Limit{
-			Count: uint64(internalLimit), // nolint:gosec // G115: limit can never reasonably exceed uint64 max // TODO: why do we use 2 x limit here?
+			Count: uint64(internalLimit), // nolint:gosec // G115: limit can never reasonably exceed uint64 max
 		},
 	}
 

pkg/solana/ccip/provider/provider.go

@@ -56,7 +56,7 @@ func NewCCIPProvider(lggr logger.Logger, chainSelector ccipocr3.ChainSelector, c
 	return &Provider{
 		lggr:  logger.Named(lggr, CCIPProviderName),
 		ca:    ca,
-		ct:    nil, // unimplemented
+		ct:    nil, // TODO: unimplemented
 		codec: c,
 	}, nil
 }