From d295948391b89118a4ad12922d86e67acf5aeb02 Mon Sep 17 00:00:00 2001 From: Aleksandr Bukata Date: Mon, 24 Feb 2025 15:38:00 +0700 Subject: [PATCH] review fixes --- execute/plugin_tokendata_test.go | 4 +- execute/test_utils.go | 2 + execute/tokendata/lbtc/attestation.go | 8 +-- execute/tokendata/lbtc/lbtc_int_test.go | 1 + execute/tokendata/usdc/attestation.go | 7 +- execute/tokendata/usdc/attestation_test.go | 1 + execute/tokendata/usdc/usdc_int_test.go | 1 + pluginconfig/token.go | 76 ++++++++++++++-------- pluginconfig/token_test.go | 76 +++++++++++++++++++++- 9 files changed, 134 insertions(+), 42 deletions(-) diff --git a/execute/plugin_tokendata_test.go b/execute/plugin_tokendata_test.go index a49a20b5b..24919e8d0 100644 --- a/execute/plugin_tokendata_test.go +++ b/execute/plugin_tokendata_test.go @@ -18,7 +18,7 @@ import ( "github.com/smartcontractkit/chainlink-ccip/execute/exectypes" "github.com/smartcontractkit/chainlink-ccip/internal/mocks/inmem" - "github.com/smartcontractkit/chainlink-ccip/pkg/ocrtypecodec" + ocrtypecodec "github.com/smartcontractkit/chainlink-ccip/pkg/ocrtypecodec/v1" readerpkg "github.com/smartcontractkit/chainlink-ccip/pkg/reader" cciptypes "github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3" ) @@ -33,7 +33,7 @@ func runRoundAndGetOutcome(ctx context.Context, ocrTypeCodec ocrtypecodec.ExecCo } func Test_LBTC_USDC_Transfer(t *testing.T) { - ocrTypeCodec := ocrtypecodec.NewExecCodecJSON() + ocrTypeCodec := ocrtypecodec.NewExecCodecProto() ctx := tests.Context(t) sourceChain := cciptypes.ChainSelector(sel.ETHEREUM_TESTNET_SEPOLIA.Selector) diff --git a/execute/test_utils.go b/execute/test_utils.go index 1ea99db90..d3a139b62 100644 --- a/execute/test_utils.go +++ b/execute/test_utils.go @@ -163,6 +163,7 @@ func (it *IntTest) WithUSDC( AttestationAPI: it.usdcServer.server.URL, AttestationAPIInterval: commonconfig.MustNewDuration(1 * time.Millisecond), AttestationAPITimeout: commonconfig.MustNewDuration(1 * time.Second), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, Tokens: map[cciptypes.ChainSelector]pluginconfig.USDCCCTPTokenConfig{ it.srcSelector: { @@ -207,6 +208,7 @@ func (it *IntTest) WithLBTC( AttestationAPI: it.lbtcServer.server.URL, AttestationAPIInterval: commonconfig.MustNewDuration(1 * time.Millisecond), AttestationAPITimeout: commonconfig.MustNewDuration(1 * time.Second), + AttestationAPICooldown: commonconfig.MustNewDuration(1 * time.Second), }, SourcePoolAddressByChain: map[cciptypes.ChainSelector]string{ it.srcSelector: sourcePoolAddress, diff --git a/execute/tokendata/lbtc/attestation.go b/execute/tokendata/lbtc/attestation.go index dec03fc3a..a05ce9fba 100644 --- a/execute/tokendata/lbtc/attestation.go +++ b/execute/tokendata/lbtc/attestation.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "maps" - "time" "github.com/smartcontractkit/chainlink-common/pkg/logger" @@ -19,9 +18,8 @@ import ( type AttestationStatus string const ( - apiVersion = "v1" - attestationPath = "deposits/getByHash" - defaultCoolDownDuration = 30 * time.Second + apiVersion = "v1" + attestationPath = "deposits/getByHash" attestationStatusUnspecified AttestationStatus = "NOTARIZATION_STATUS_UNSPECIFIED" attestationStatusFailed AttestationStatus = "NOTARIZATION_STATUS_FAILED" @@ -62,7 +60,7 @@ func NewLBTCAttestationClient( config.AttestationAPI, config.AttestationAPIInterval.Duration(), config.AttestationAPITimeout.Duration(), - defaultCoolDownDuration, + config.AttestationAPICooldown.Duration(), ) if err != nil { return nil, fmt.Errorf("get http client: %w", err) diff --git a/execute/tokendata/lbtc/lbtc_int_test.go b/execute/tokendata/lbtc/lbtc_int_test.go index 6f4c12339..7c21fb3cf 100644 --- a/execute/tokendata/lbtc/lbtc_int_test.go +++ b/execute/tokendata/lbtc/lbtc_int_test.go @@ -196,6 +196,7 @@ func Test_LBTC_Flow(t *testing.T) { AttestationAPI: server.URL, AttestationAPIInterval: commonconfig.MustNewDuration(1 * time.Microsecond), AttestationAPITimeout: commonconfig.MustNewDuration(1 * time.Second), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, SourcePoolAddressByChain: map[cciptypes.ChainSelector]string{ bscChain: bscPool, diff --git a/execute/tokendata/usdc/attestation.go b/execute/tokendata/usdc/attestation.go index 70f77a419..908b841a7 100644 --- a/execute/tokendata/usdc/attestation.go +++ b/execute/tokendata/usdc/attestation.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "time" "github.com/smartcontractkit/chainlink-common/pkg/hashutil" "github.com/smartcontractkit/chainlink-common/pkg/logger" @@ -24,10 +23,6 @@ const ( apiVersion = "v1" attestationPath = "attestations" - // defaultCoolDownDurationSec defines the default time to wait after getting rate limited. - // this value is only used if the 429 response does not contain the Retry-After header - defaultCoolDownDuration = 5 * time.Minute - attestationStatusSuccess attestationStatus = "complete" attestationStatusPending attestationStatus = "pending_confirmations" ) @@ -93,7 +88,7 @@ func NewSequentialAttestationClient( config.AttestationAPI, config.AttestationAPIInterval.Duration(), config.AttestationAPITimeout.Duration(), - defaultCoolDownDuration, + config.AttestationAPICooldown.Duration(), ) if err != nil { return nil, fmt.Errorf("create HTTP client: %w", err) diff --git a/execute/tokendata/usdc/attestation_test.go b/execute/tokendata/usdc/attestation_test.go index 62ab0e327..f8c7f06c0 100644 --- a/execute/tokendata/usdc/attestation_test.go +++ b/execute/tokendata/usdc/attestation_test.go @@ -183,6 +183,7 @@ func Test_AttestationClient(t *testing.T) { AttestationAPI: server.URL, AttestationAPIInterval: commonconfig.MustNewDuration(1 * time.Millisecond), AttestationAPITimeout: commonconfig.MustNewDuration(5 * time.Second), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, }, ) diff --git a/execute/tokendata/usdc/usdc_int_test.go b/execute/tokendata/usdc/usdc_int_test.go index a7141036e..422d52e29 100644 --- a/execute/tokendata/usdc/usdc_int_test.go +++ b/execute/tokendata/usdc/usdc_int_test.go @@ -235,6 +235,7 @@ func Test_USDC_CCTP_Flow(t *testing.T) { AttestationAPI: server.URL, AttestationAPIInterval: commonconfig.MustNewDuration(1 * time.Microsecond), AttestationAPITimeout: commonconfig.MustNewDuration(1 * time.Second), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, Tokens: map[cciptypes.ChainSelector]pluginconfig.USDCCCTPTokenConfig{ fujiChain: { diff --git a/pluginconfig/token.go b/pluginconfig/token.go index 304949608..a262b545b 100644 --- a/pluginconfig/token.go +++ b/pluginconfig/token.go @@ -77,10 +77,19 @@ func (t *TokenDataObserverConfig) WellFormed() error { // Validate checks that the observer's config is semantically correct - fields are set correctly // depending on the config's type func (t *TokenDataObserverConfig) Validate() error { + if err := t.WellFormed(); err != nil { + return err + } if t.IsUSDC() { + if t.LBTCObserverConfig != nil { + return errors.New("LBTCObserverConfig must be null with USDC plugin type") + } return t.USDCCCTPObserverConfig.Validate() } if t.IsLBTC() { + if t.USDCCCTPObserverConfig != nil { + return errors.New("USDCCCTPObserverConfig must be null with LBTC plugin type") + } return t.LBTCObserverConfig.Validate() } return errors.New("unknown token data observer type " + t.Type) @@ -98,31 +107,30 @@ func (t *TokenDataObserverConfig) IsLBTC() bool { // It constructs raw map based on provided type. Custom marshaller is needed because default golang marshaller // doesn't marshal clashing fields of pointer embeddings even if only one pointer is present and rest are set to nil func (t *TokenDataObserverConfig) MarshalJSON() ([]byte, error) { - raw := map[string]interface{}{ - "type": t.Type, - "version": t.Version, - } - var err error - var configJSONRaw []byte switch t.Type { case USDCCCTPHandlerType: - configJSONRaw, err = json.Marshal(t.USDCCCTPObserverConfig) - if err != nil { - return nil, err - } + return json.Marshal(&struct { + Type string `json:"type"` + Version string `json:"version"` + *USDCCCTPObserverConfig + }{ + Type: t.Type, + Version: t.Version, + USDCCCTPObserverConfig: t.USDCCCTPObserverConfig, + }) case LBTCHandlerType: - configJSONRaw, err = json.Marshal(t.LBTCObserverConfig) - if err != nil { - return nil, err - } + return json.Marshal(&struct { + Type string `json:"type"` + Version string `json:"version"` + *LBTCObserverConfig + }{ + Type: t.Type, + Version: t.Version, + LBTCObserverConfig: t.LBTCObserverConfig, + }) default: return nil, fmt.Errorf("unknown token data observer type: %q", t.Type) } - err = json.Unmarshal(configJSONRaw, &raw) - if err != nil { - return nil, err - } - return json.Marshal(raw) } // UnmarshalJSON is a custom JSON unmarshaller for TokenDataObserverConfig. @@ -167,6 +175,9 @@ type AttestationConfig struct { // AttestationAPIInterval defines the rate in requests per second that the attestation API can be called. // Default set according to the APIs documentated 10 requests per second rate limit. AttestationAPIInterval *commonconfig.Duration `json:"attestationAPIInterval"` + // AttestationAPICooldown defines in what time it is allowed to make next call to API. + // Activates when plugin hits API's rate limits + AttestationAPICooldown *commonconfig.Duration `json:"attestationAPICooldown"` } func (p *AttestationConfig) setDefaults() { @@ -251,7 +262,14 @@ type USDCCCTPObserverConfig struct { Tokens map[cciptypes.ChainSelector]USDCCCTPTokenConfig `json:"tokens"` } +func (p *USDCCCTPObserverConfig) setDefaults() { + if p.AttestationAPICooldown == nil || p.AttestationAPICooldown.Duration() == 0 { + p.AttestationAPICooldown = commonconfig.MustNewDuration(5 * time.Minute) + } +} + func (p *USDCCCTPObserverConfig) Validate() error { + p.setDefaults() err := p.AttestationConfig.Validate() if err != nil { return err @@ -260,7 +278,6 @@ func (p *USDCCCTPObserverConfig) Validate() error { if err != nil { return err } - if len(p.Tokens) == 0 { return errors.New("Tokens not set") } @@ -299,20 +316,15 @@ type LBTCObserverConfig struct { } func (c *LBTCObserverConfig) setDefaults() { + if c.AttestationAPICooldown == nil || c.AttestationAPICooldown.Duration() == 0 { + c.AttestationAPICooldown = commonconfig.MustNewDuration(1 * time.Second) + } if c.AttestationAPIBatchSize == 0 { c.AttestationAPIBatchSize = 50 } } func (c *LBTCObserverConfig) Validate() error { - err := c.AttestationConfig.Validate() - if err != nil { - return err - } - err = c.WorkerConfig.Validate() - if err != nil { - return err - } c.setDefaults() if c.AttestationAPIBatchSize == 0 { return errors.New("AttestationAPIBatchSize is not set") @@ -325,5 +337,13 @@ func (c *LBTCObserverConfig) Validate() error { return errors.New("SourcePoolAddressByChain is empty") } } + err := c.AttestationConfig.Validate() + if err != nil { + return err + } + err = c.WorkerConfig.Validate() + if err != nil { + return err + } return nil } diff --git a/pluginconfig/token_test.go b/pluginconfig/token_test.go index 3cf8efdd9..73c90710a 100644 --- a/pluginconfig/token_test.go +++ b/pluginconfig/token_test.go @@ -82,6 +82,7 @@ func Test_TokenDataObserver_Unmarshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10m0s", "numWorkers": 10, "cacheExpirationInterval": "5s", "cacheCleanupInterval": "6s", @@ -97,6 +98,7 @@ func Test_TokenDataObserver_Unmarshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Minute), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -132,7 +134,8 @@ func Test_TokenDataObserver_Unmarshal(t *testing.T) { }, "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", - "attestationAPIInterval": "500ms" + "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10m0s" } ],`, want: []TokenDataObserverConfig{ @@ -144,6 +147,7 @@ func Test_TokenDataObserver_Unmarshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Minute), }, Tokens: map[cciptypes.ChainSelector]USDCCCTPTokenConfig{ 1: { @@ -217,6 +221,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "", "attestationAPIInterval": null, "attestationAPITimeout": null, + "attestationAPICooldown": null, "numWorkers": 0, "observeTimeout": null, "cacheCleanupInterval": null, @@ -226,6 +231,33 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { ]`, wantErr: false, }, + { + name: "empty lbtc is set", + config: []TokenDataObserverConfig{ + { + Type: "lbtc", + Version: "1.0", + LBTCObserverConfig: &LBTCObserverConfig{}, + }, + }, + wantJSON: `[ + { + "type": "lbtc", + "version": "1.0", + "attestationAPI": "", + "attestationAPIInterval": null, + "attestationAPITimeout": null, + "attestationAPICooldown": null, + "numWorkers": 0, + "observeTimeout": null, + "cacheCleanupInterval": null, + "cacheExpirationInterval": null, + "attestationAPIBatchSize": 0, + "sourcePoolAddressByChain": null + } + ]`, + wantErr: false, + }, { name: "valid config with USDCCCTPObserverConfig", config: []TokenDataObserverConfig{ @@ -237,6 +269,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Minute), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -266,6 +299,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10m0s", "numWorkers": 10, "cacheExpirationInterval": "5s", "cacheCleanupInterval": "6s", @@ -284,6 +318,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Minute), }, Tokens: map[cciptypes.ChainSelector]USDCCCTPTokenConfig{ 1: { @@ -315,6 +350,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10m0s", "numWorkers": 0, "observeTimeout": null, "cacheCleanupInterval": null, @@ -333,6 +369,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Second), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -356,6 +393,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10s", "attestationAPIBatchSize": 0, "numWorkers": 10, "cacheExpirationInterval": "5s", @@ -375,6 +413,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Minute), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -402,6 +441,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(10 * time.Second), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -432,6 +472,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10m0s", "numWorkers": 10, "cacheExpirationInterval": "5s", "cacheCleanupInterval": "6s", @@ -446,6 +487,7 @@ func Test_TokenDataObserver_Marshal(t *testing.T) { "attestationAPI": "http://localhost:8080", "attestationAPITimeout": "1s", "attestationAPIInterval": "500ms", + "attestationAPICooldown": "10s", "attestationAPIBatchSize": 0, "numWorkers": 10, "cacheExpirationInterval": "5s", @@ -496,6 +538,7 @@ func Test_TokenDataObserver_Validation(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -518,6 +561,7 @@ func Test_TokenDataObserver_Validation(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(1 * time.Second), }, WorkerConfig: WorkerConfig{ NumWorkers: 10, @@ -579,6 +623,7 @@ func Test_TokenDataObserver_Validation(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(5 * time.Minute), }, }, }), @@ -598,6 +643,7 @@ func Test_TokenDataObserver_Validation(t *testing.T) { AttestationAPI: "http://localhost:8080", AttestationAPITimeout: commonconfig.MustNewDuration(time.Second), AttestationAPIInterval: commonconfig.MustNewDuration(500 * time.Millisecond), + AttestationAPICooldown: commonconfig.MustNewDuration(1 * time.Second), }, }, }), @@ -624,6 +670,34 @@ func Test_TokenDataObserver_Validation(t *testing.T) { wantErr: true, errMsg: "duplicate token data observer type", }, + { + name: "usdc type but two simultaneous configs", + config: withBaseConfig( + TokenDataObserverConfig{ + Type: "usdc-cctp", + Version: "1.0", + USDCCCTPObserverConfig: withUSDCConfig(), + LBTCObserverConfig: withLBTCConfig(), + }), + usdcEnabled: true, + lbtcEnabled: false, + wantErr: true, + errMsg: "LBTCObserverConfig must be null with USDC plugin type", + }, + { + name: "lbtc type but two simultaneous configs", + config: withBaseConfig( + TokenDataObserverConfig{ + Type: "lbtc", + Version: "1.0", + USDCCCTPObserverConfig: withUSDCConfig(), + LBTCObserverConfig: withLBTCConfig(), + }), + usdcEnabled: false, + lbtcEnabled: true, + wantErr: true, + errMsg: "USDCCCTPObserverConfig must be null with LBTC plugin type", + }, { name: "valid config with multiple the same observers types but different versions", config: withBaseConfig(