From f2d5f603f583f4095804b8dbf6aaf105c6d9981d Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 9 Nov 2023 17:42:19 +0100 Subject: [PATCH 01/16] Create Evm Transaction Endpoint --- core/store/models/common.go | 14 ++ core/web/evm_transactions_controller.go | 106 +++++++++++++++ core/web/evm_transactions_controller_test.go | 136 +++++++++++++++++++ core/web/router.go | 1 + 4 files changed, 257 insertions(+) diff --git a/core/store/models/common.go b/core/store/models/common.go index fc7f7762046..e0311430d91 100644 --- a/core/store/models/common.go +++ b/core/store/models/common.go @@ -662,3 +662,17 @@ func (h ServiceHeader) Validate() (err error) { } return } + +// CreateEVMTransactionRequest represents a request to create request to submit evm transaction. +type CreateEVMTransactionRequest struct { + IdempotencyKey string `json:"idempotencyKey"` + ChainID *utils.Big `json:"chainID"` + DestinationAddress common.Address `json:"destinationAddress"` + FromAddress common.Address `json:"fromAddress"` // optional, if not set we use one of accounts available for specified chain + EncodedPayload string `json:"encodedPayload"` // hex encoded payload + Value utils.Big `json:"value"` + ForwarderAddress common.Address `json:"forwarderAddress"` + FeeLimit uint32 `json:"feeLimit"` + SkipWaitTxAttempt bool `json:"skipWaitTxAttempt"` + WaitAttemptTimeout *time.Duration `json:"waitAttemptTimeout"` +} diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 2b2fd2d554f..c4b93743bb8 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -2,9 +2,18 @@ package web import ( "database/sql" + "fmt" "net/http" + "time" + "github.com/ethereum/go-ethereum/common/hexutil" + + txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + "github.com/smartcontractkit/chainlink/v2/core/logger/audit" "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" + "github.com/smartcontractkit/chainlink/v2/core/store/models" + "github.com/smartcontractkit/chainlink/v2/core/utils" "github.com/smartcontractkit/chainlink/v2/core/web/presenters" "github.com/ethereum/go-ethereum/common" @@ -47,3 +56,100 @@ func (tc *TransactionsController) Show(c *gin.Context) { jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(*ethTxAttempt), "transaction") } + +// Create signs and sends transaction from specified address. If address is not provided uses one of enabled keys for +// specified chain. +func (tc *TransactionsController) Create(c *gin.Context) { + var tx models.CreateEVMTransactionRequest + if err := c.ShouldBindJSON(&tx); err != nil { + jsonAPIError(c, http.StatusBadRequest, err) + return + } + + if tx.IdempotencyKey == "" { + jsonAPIError(c, http.StatusBadRequest, errors.New("idempotencyKey must be set")) + return + } + + decoded, err := hexutil.Decode(tx.EncodedPayload) + if err != nil { + jsonAPIError(c, http.StatusBadRequest, errors.Errorf("encodedPayload is malformed: %v", err)) + return + } + + if tx.ChainID == nil { + jsonAPIError(c, http.StatusBadRequest, errors.New("chainID must be set")) + return + } + + chain, err := getChain(tc.App.GetRelayers().LegacyEVMChains(), tx.ChainID.String()) + if err != nil { + if errors.Is(err, ErrMissingChainID) { + jsonAPIError(c, http.StatusUnprocessableEntity, err) + return + } + + tc.App.GetLogger().Errorf("Failed to get chain", "err", err) + jsonAPIError(c, http.StatusInternalServerError, err) + return + } + + if tx.FromAddress == utils.ZeroAddress { + tx.FromAddress, err = tc.App.GetKeyStore().Eth().GetRoundRobinAddress(tx.ChainID.ToInt()) + if err != nil { + jsonAPIError(c, http.StatusUnprocessableEntity, errors.Errorf("failed to get fromAddress: %v", err)) + return + } + } else { + _, err = tc.App.GetKeyStore().Eth().GetRoundRobinAddress(tx.ChainID.ToInt(), tx.FromAddress) + if err != nil { + jsonAPIError(c, http.StatusUnprocessableEntity, + errors.Errorf("fromAddress %v is not available: %v", tx.FromAddress, err)) + return + } + } + + if tx.FeeLimit == 0 { + // TODO: is it a right place to get default limit? + tx.FeeLimit = chain.Config().EVM().GasEstimator().LimitDefault() + } + + value := tx.Value.ToInt() + etx, err := chain.TxManager().CreateTransaction(c, txmgr.TxRequest{ + IdempotencyKey: &tx.IdempotencyKey, + FromAddress: tx.FromAddress, + ToAddress: tx.DestinationAddress, + EncodedPayload: decoded, + Value: *value, + FeeLimit: tx.FeeLimit, + ForwarderAddress: tx.ForwarderAddress, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }) + if err != nil { + jsonAPIError(c, http.StatusBadRequest, errors.Errorf("transaction failed: %v", err)) + return + } + + tc.App.GetAuditLogger().Audit(audit.EthTransactionCreated, map[string]interface{}{ + "ethTX": etx, + }) + + // skip waiting for txmgr to create TxAttempt + if tx.SkipWaitTxAttempt { + jsonAPIResponse(c, presenters.NewEthTxResource(etx), "eth_tx") + return + } + + timeout := 10 * time.Second // default + if tx.WaitAttemptTimeout != nil { + timeout = *tx.WaitAttemptTimeout + } + + // wait and retrieve tx attempt matching tx ID + attempt, err := FindTxAttempt(c, timeout, etx, tc.App.TxmStorageService().FindTxWithAttempts) + if err != nil { + jsonAPIError(c, http.StatusGatewayTimeout, fmt.Errorf("failed to find transaction within timeout: %w", err)) + return + } + jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(attempt), "eth_tx") +} diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 9d8336325e2..d2984aa17f2 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -1,15 +1,21 @@ package web_test import ( + "bytes" + "encoding/json" "fmt" "net/http" "testing" + "github.com/ethereum/go-ethereum/common" + txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/assets" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/store/models" + "github.com/smartcontractkit/chainlink/v2/core/utils" "github.com/smartcontractkit/chainlink/v2/core/web" "github.com/smartcontractkit/chainlink/v2/core/web/presenters" @@ -125,3 +131,133 @@ func TestTransactionsController_Show_NotFound(t *testing.T) { t.Cleanup(cleanup) cltest.AssertServerResponse(t, resp, http.StatusNotFound) } + +const txCreatePath = "/v2/transactions/evm" + +// TestTransactionsController_Create_Stateless_Validations - tests Create endpoint of TestTransactionsController that +// do not require different state/configuration of the application. +func TestTransactionsController_Create_Stateless_Validations(t *testing.T) { + t.Parallel() + + app := cltest.NewApplication(t) + require.NoError(t, app.Start(testutils.Context(t))) + + client := app.NewHTTPClient(nil) + + t.Run("Fails on malformed json", func(t *testing.T) { + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer([]byte("Hello"))) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + }) + t.Run("Fails on missing Idempotency key", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "idempotencyKey must be set", respError.Error()) + }) + t.Run("Fails on malformed payload", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "encodedPayload is malformed: empty hex string", respError.Error()) + }) + t.Run("Fails if chain ID is not set", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "chainID must be set", respError.Error()) + }) + t.Run("Fails on requesting chain that is not available", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: utils.NewBigI(1), + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, web.ErrMissingChainID.Error(), respError.Error()) + }) + t.Run("Fails when fromAddress is not specified and there are no available keys ", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: utils.NewBigI(0), + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "failed to get fromAddress: no sending keys available for chain 0", respError.Error()) + }) + t.Run("Fails when specified fromAddress is not available for the chain ", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + FromAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: utils.NewBigI(0), + } + + body, err := json.Marshal(&request) + assert.NoError(t, err) + + resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) + t.Cleanup(cleanup) + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, + "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: no sending "+ + "keys available for chain 0 that match whitelist: [0xfa01fA015c8A5332987319823728982379128371]", + respError.Error()) + }) +} diff --git a/core/web/router.go b/core/web/router.go index 28bd4f2170c..fe79f08d673 100644 --- a/core/web/router.go +++ b/core/web/router.go @@ -279,6 +279,7 @@ func v2Routes(app chainlink.Application, r *gin.RouterGroup) { txs := TransactionsController{app} authv2.GET("/transactions/evm", paginatedRequest(txs.Index)) + authv2.POST("/transactions/evm", txs.Create) authv2.GET("/transactions/evm/:TxHash", txs.Show) authv2.GET("/transactions", paginatedRequest(txs.Index)) authv2.GET("/transactions/:TxHash", txs.Show) From 0213c09c32658b207c072815a3814338cf42802d Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 9 Nov 2023 21:09:14 +0100 Subject: [PATCH 02/16] rewrite tests to use mocks --- core/chains/evm/config/config.go | 1 + core/chains/evm/config/mocks/evm.go | 456 +++++++++++++++++++ core/store/models/common.go | 20 +- core/web/evm_transactions_controller.go | 31 +- core/web/evm_transactions_controller_test.go | 222 ++++++--- core/web/router.go | 3 +- 6 files changed, 651 insertions(+), 82 deletions(-) create mode 100644 core/chains/evm/config/mocks/evm.go diff --git a/core/chains/evm/config/config.go b/core/chains/evm/config/config.go index f8ec030969e..ac13db26886 100644 --- a/core/chains/evm/config/config.go +++ b/core/chains/evm/config/config.go @@ -10,6 +10,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/config" ) +//go:generate mockery --quiet --name EVM --output ./mocks/ --case=underscore type EVM interface { HeadTracker() HeadTracker BalanceMonitor() BalanceMonitor diff --git a/core/chains/evm/config/mocks/evm.go b/core/chains/evm/config/mocks/evm.go new file mode 100644 index 00000000000..746ee8cbe51 --- /dev/null +++ b/core/chains/evm/config/mocks/evm.go @@ -0,0 +1,456 @@ +// Code generated by mockery v2.35.4. DO NOT EDIT. + +package mocks + +import ( + big "math/big" + + assets "github.com/smartcontractkit/chainlink/v2/core/assets" + + config "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + + coreconfig "github.com/smartcontractkit/chainlink/v2/core/config" + + mock "github.com/stretchr/testify/mock" + + time "time" +) + +// EVM is an autogenerated mock type for the EVM type +type EVM struct { + mock.Mock +} + +// AutoCreateKey provides a mock function with given fields: +func (_m *EVM) AutoCreateKey() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// BalanceMonitor provides a mock function with given fields: +func (_m *EVM) BalanceMonitor() config.BalanceMonitor { + ret := _m.Called() + + var r0 config.BalanceMonitor + if rf, ok := ret.Get(0).(func() config.BalanceMonitor); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.BalanceMonitor) + } + } + + return r0 +} + +// BlockBackfillDepth provides a mock function with given fields: +func (_m *EVM) BlockBackfillDepth() uint64 { + ret := _m.Called() + + var r0 uint64 + if rf, ok := ret.Get(0).(func() uint64); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint64) + } + + return r0 +} + +// BlockBackfillSkip provides a mock function with given fields: +func (_m *EVM) BlockBackfillSkip() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// BlockEmissionIdleWarningThreshold provides a mock function with given fields: +func (_m *EVM) BlockEmissionIdleWarningThreshold() time.Duration { + ret := _m.Called() + + var r0 time.Duration + if rf, ok := ret.Get(0).(func() time.Duration); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(time.Duration) + } + + return r0 +} + +// ChainID provides a mock function with given fields: +func (_m *EVM) ChainID() *big.Int { + ret := _m.Called() + + var r0 *big.Int + if rf, ok := ret.Get(0).(func() *big.Int); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*big.Int) + } + } + + return r0 +} + +// ChainType provides a mock function with given fields: +func (_m *EVM) ChainType() coreconfig.ChainType { + ret := _m.Called() + + var r0 coreconfig.ChainType + if rf, ok := ret.Get(0).(func() coreconfig.ChainType); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(coreconfig.ChainType) + } + + return r0 +} + +// FinalityDepth provides a mock function with given fields: +func (_m *EVM) FinalityDepth() uint32 { + ret := _m.Called() + + var r0 uint32 + if rf, ok := ret.Get(0).(func() uint32); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint32) + } + + return r0 +} + +// FinalityTagEnabled provides a mock function with given fields: +func (_m *EVM) FinalityTagEnabled() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// FlagsContractAddress provides a mock function with given fields: +func (_m *EVM) FlagsContractAddress() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// GasEstimator provides a mock function with given fields: +func (_m *EVM) GasEstimator() config.GasEstimator { + ret := _m.Called() + + var r0 config.GasEstimator + if rf, ok := ret.Get(0).(func() config.GasEstimator); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.GasEstimator) + } + } + + return r0 +} + +// HeadTracker provides a mock function with given fields: +func (_m *EVM) HeadTracker() config.HeadTracker { + ret := _m.Called() + + var r0 config.HeadTracker + if rf, ok := ret.Get(0).(func() config.HeadTracker); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.HeadTracker) + } + } + + return r0 +} + +// IsEnabled provides a mock function with given fields: +func (_m *EVM) IsEnabled() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// LinkContractAddress provides a mock function with given fields: +func (_m *EVM) LinkContractAddress() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// LogBackfillBatchSize provides a mock function with given fields: +func (_m *EVM) LogBackfillBatchSize() uint32 { + ret := _m.Called() + + var r0 uint32 + if rf, ok := ret.Get(0).(func() uint32); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint32) + } + + return r0 +} + +// LogKeepBlocksDepth provides a mock function with given fields: +func (_m *EVM) LogKeepBlocksDepth() uint32 { + ret := _m.Called() + + var r0 uint32 + if rf, ok := ret.Get(0).(func() uint32); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint32) + } + + return r0 +} + +// LogPollInterval provides a mock function with given fields: +func (_m *EVM) LogPollInterval() time.Duration { + ret := _m.Called() + + var r0 time.Duration + if rf, ok := ret.Get(0).(func() time.Duration); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(time.Duration) + } + + return r0 +} + +// MinContractPayment provides a mock function with given fields: +func (_m *EVM) MinContractPayment() *assets.Link { + ret := _m.Called() + + var r0 *assets.Link + if rf, ok := ret.Get(0).(func() *assets.Link); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*assets.Link) + } + } + + return r0 +} + +// MinIncomingConfirmations provides a mock function with given fields: +func (_m *EVM) MinIncomingConfirmations() uint32 { + ret := _m.Called() + + var r0 uint32 + if rf, ok := ret.Get(0).(func() uint32); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint32) + } + + return r0 +} + +// NodeNoNewHeadsThreshold provides a mock function with given fields: +func (_m *EVM) NodeNoNewHeadsThreshold() time.Duration { + ret := _m.Called() + + var r0 time.Duration + if rf, ok := ret.Get(0).(func() time.Duration); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(time.Duration) + } + + return r0 +} + +// NodePool provides a mock function with given fields: +func (_m *EVM) NodePool() config.NodePool { + ret := _m.Called() + + var r0 config.NodePool + if rf, ok := ret.Get(0).(func() config.NodePool); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.NodePool) + } + } + + return r0 +} + +// NonceAutoSync provides a mock function with given fields: +func (_m *EVM) NonceAutoSync() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// OCR provides a mock function with given fields: +func (_m *EVM) OCR() config.OCR { + ret := _m.Called() + + var r0 config.OCR + if rf, ok := ret.Get(0).(func() config.OCR); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.OCR) + } + } + + return r0 +} + +// OCR2 provides a mock function with given fields: +func (_m *EVM) OCR2() config.OCR2 { + ret := _m.Called() + + var r0 config.OCR2 + if rf, ok := ret.Get(0).(func() config.OCR2); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.OCR2) + } + } + + return r0 +} + +// OperatorFactoryAddress provides a mock function with given fields: +func (_m *EVM) OperatorFactoryAddress() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// RPCDefaultBatchSize provides a mock function with given fields: +func (_m *EVM) RPCDefaultBatchSize() uint32 { + ret := _m.Called() + + var r0 uint32 + if rf, ok := ret.Get(0).(func() uint32); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint32) + } + + return r0 +} + +// TOMLString provides a mock function with given fields: +func (_m *EVM) TOMLString() (string, error) { + ret := _m.Called() + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func() (string, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Transactions provides a mock function with given fields: +func (_m *EVM) Transactions() config.Transactions { + ret := _m.Called() + + var r0 config.Transactions + if rf, ok := ret.Get(0).(func() config.Transactions); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.Transactions) + } + } + + return r0 +} + +// NewEVM creates a new instance of EVM. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewEVM(t interface { + mock.TestingT + Cleanup(func()) +}) *EVM { + mock := &EVM{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/store/models/common.go b/core/store/models/common.go index e0311430d91..0b35062dc94 100644 --- a/core/store/models/common.go +++ b/core/store/models/common.go @@ -665,14 +665,14 @@ func (h ServiceHeader) Validate() (err error) { // CreateEVMTransactionRequest represents a request to create request to submit evm transaction. type CreateEVMTransactionRequest struct { - IdempotencyKey string `json:"idempotencyKey"` - ChainID *utils.Big `json:"chainID"` - DestinationAddress common.Address `json:"destinationAddress"` - FromAddress common.Address `json:"fromAddress"` // optional, if not set we use one of accounts available for specified chain - EncodedPayload string `json:"encodedPayload"` // hex encoded payload - Value utils.Big `json:"value"` - ForwarderAddress common.Address `json:"forwarderAddress"` - FeeLimit uint32 `json:"feeLimit"` - SkipWaitTxAttempt bool `json:"skipWaitTxAttempt"` - WaitAttemptTimeout *time.Duration `json:"waitAttemptTimeout"` + IdempotencyKey string `json:"idempotencyKey"` + ChainID *utils.Big `json:"chainID"` + ToAddress *common.Address `json:"toAddress"` + FromAddress common.Address `json:"fromAddress"` // optional, if not set we use one of accounts available for specified chain + EncodedPayload string `json:"encodedPayload"` // hex encoded payload + Value *utils.Big `json:"value"` + ForwarderAddress common.Address `json:"forwarderAddress"` + FeeLimit uint32 `json:"feeLimit"` + SkipWaitTxAttempt bool `json:"skipWaitTxAttempt"` + WaitAttemptTimeout *time.Duration `json:"waitAttemptTimeout"` } diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index c4b93743bb8..470d5355faa 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -9,9 +9,11 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" "github.com/smartcontractkit/chainlink/v2/core/logger/audit" "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" + "github.com/smartcontractkit/chainlink/v2/core/services/keystore" "github.com/smartcontractkit/chainlink/v2/core/store/models" "github.com/smartcontractkit/chainlink/v2/core/utils" "github.com/smartcontractkit/chainlink/v2/core/web/presenters" @@ -57,9 +59,23 @@ func (tc *TransactionsController) Show(c *gin.Context) { jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(*ethTxAttempt), "transaction") } +type EvmTransactionController struct { + App chainlink.Application + Chains evm.LegacyChainContainer + KeyStore keystore.Eth +} + +func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { + return &EvmTransactionController{ + App: app, + Chains: app.GetRelayers().LegacyEVMChains(), + KeyStore: app.GetKeyStore().Eth(), + } +} + // Create signs and sends transaction from specified address. If address is not provided uses one of enabled keys for // specified chain. -func (tc *TransactionsController) Create(c *gin.Context) { +func (tc *EvmTransactionController) Create(c *gin.Context) { var tx models.CreateEVMTransactionRequest if err := c.ShouldBindJSON(&tx); err != nil { jsonAPIError(c, http.StatusBadRequest, err) @@ -82,7 +98,12 @@ func (tc *TransactionsController) Create(c *gin.Context) { return } - chain, err := getChain(tc.App.GetRelayers().LegacyEVMChains(), tx.ChainID.String()) + if tx.ToAddress == nil { + jsonAPIError(c, http.StatusBadRequest, errors.New("toAddress must be set")) + return + } + + chain, err := getChain(tc.Chains, tx.ChainID.String()) if err != nil { if errors.Is(err, ErrMissingChainID) { jsonAPIError(c, http.StatusUnprocessableEntity, err) @@ -95,13 +116,13 @@ func (tc *TransactionsController) Create(c *gin.Context) { } if tx.FromAddress == utils.ZeroAddress { - tx.FromAddress, err = tc.App.GetKeyStore().Eth().GetRoundRobinAddress(tx.ChainID.ToInt()) + tx.FromAddress, err = tc.KeyStore.GetRoundRobinAddress(tx.ChainID.ToInt()) if err != nil { jsonAPIError(c, http.StatusUnprocessableEntity, errors.Errorf("failed to get fromAddress: %v", err)) return } } else { - _, err = tc.App.GetKeyStore().Eth().GetRoundRobinAddress(tx.ChainID.ToInt(), tx.FromAddress) + _, err = tc.KeyStore.GetRoundRobinAddress(tx.ChainID.ToInt(), tx.FromAddress) if err != nil { jsonAPIError(c, http.StatusUnprocessableEntity, errors.Errorf("fromAddress %v is not available: %v", tx.FromAddress, err)) @@ -118,7 +139,7 @@ func (tc *TransactionsController) Create(c *gin.Context) { etx, err := chain.TxManager().CreateTransaction(c, txmgr.TxRequest{ IdempotencyKey: &tx.IdempotencyKey, FromAddress: tx.FromAddress, - ToAddress: tx.DestinationAddress, + ToAddress: *tx.ToAddress, EncodedPayload: decoded, Value: *value, FeeLimit: tx.FeeLimit, diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index d2984aa17f2..890563be6b1 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -4,16 +4,31 @@ import ( "bytes" "encoding/json" "fmt" + "math/big" "net/http" + "net/http/httptest" "testing" + "github.com/cometbft/cometbft/libs/rand" "github.com/ethereum/go-ethereum/common" + "github.com/gin-gonic/gin" + "github.com/pkg/errors" + "github.com/status-im/keycard-go/hexutils" + "github.com/stretchr/testify/mock" + txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmMocks "github.com/smartcontractkit/chainlink/v2/common/txmgr/mocks" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/assets" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm" + evmConfigMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + ksMocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" "github.com/smartcontractkit/chainlink/v2/core/store/models" "github.com/smartcontractkit/chainlink/v2/core/utils" "github.com/smartcontractkit/chainlink/v2/core/web" @@ -132,35 +147,33 @@ func TestTransactionsController_Show_NotFound(t *testing.T) { cltest.AssertServerResponse(t, resp, http.StatusNotFound) } -const txCreatePath = "/v2/transactions/evm" - -// TestTransactionsController_Create_Stateless_Validations - tests Create endpoint of TestTransactionsController that -// do not require different state/configuration of the application. -func TestTransactionsController_Create_Stateless_Validations(t *testing.T) { +func TestTransactionsController_Create(t *testing.T) { t.Parallel() + const txCreatePath = "/v2/transactions/evm" - app := cltest.NewApplication(t) - require.NoError(t, app.Start(testutils.Context(t))) + createTx := func(controller *web.EvmTransactionController, request interface{}) *httptest.ResponseRecorder { + body, err := json.Marshal(&request) + assert.NoError(t, err) - client := app.NewHTTPClient(nil) + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPost, txCreatePath, bytes.NewBuffer(body)) + router := gin.New() + router.POST(txCreatePath, controller.Create) + router.ServeHTTP(w, req) + return w + } t.Run("Fails on malformed json", func(t *testing.T) { - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer([]byte("Hello"))) - t.Cleanup(cleanup) + resp := createTx(&web.EvmTransactionController{}, "Hello") - cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + cltest.AssertServerResponse(t, resp.Result(), http.StatusBadRequest) }) t.Run("Fails on missing Idempotency key", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), } - body, err := json.Marshal(&request) - assert.NoError(t, err) - - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + resp := createTx(&web.EvmTransactionController{}, request).Result() cltest.AssertServerResponse(t, resp, http.StatusBadRequest) respError := cltest.ParseJSONAPIErrors(t, resp.Body) @@ -168,16 +181,12 @@ func TestTransactionsController_Create_Stateless_Validations(t *testing.T) { }) t.Run("Fails on malformed payload", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), - IdempotencyKey: "idempotency_key", + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", } - body, err := json.Marshal(&request) - assert.NoError(t, err) - - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + resp := createTx(&web.EvmTransactionController{}, request).Result() cltest.AssertServerResponse(t, resp, http.StatusBadRequest) respError := cltest.ParseJSONAPIErrors(t, resp.Body) @@ -185,36 +194,49 @@ func TestTransactionsController_Create_Stateless_Validations(t *testing.T) { }) t.Run("Fails if chain ID is not set", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x", + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", } - body, err := json.Marshal(&request) - assert.NoError(t, err) - - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + resp := createTx(&web.EvmTransactionController{}, request).Result() cltest.AssertServerResponse(t, resp, http.StatusBadRequest) respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, "chainID must be set", respError.Error()) }) - t.Run("Fails on requesting chain that is not available", func(t *testing.T) { + t.Run("Fails if toAddress is not specified", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x", - ChainID: utils.NewBigI(1), + ToAddress: nil, + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: utils.NewBigI(0), } - body, err := json.Marshal(&request) - assert.NoError(t, err) + resp := createTx(&web.EvmTransactionController{}, request).Result() - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "toAddress must be set", respError.Error()) + }) + chainID := utils.NewBigI(rand.Int64()) + t.Run("Fails if requested chain that is not available", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chainContainer.On("Get", chainID.String()).Return(nil, web.ErrMissingChainID).Once() + controller := &web.EvmTransactionController{ + Chains: chainContainer, + } + resp := createTx(controller, request).Result() cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) respError := cltest.ParseJSONAPIErrors(t, resp.Body) @@ -222,42 +244,110 @@ func TestTransactionsController_Create_Stateless_Validations(t *testing.T) { }) t.Run("Fails when fromAddress is not specified and there are no available keys ", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x", - ChainID: utils.NewBigI(0), + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, } - body, err := json.Marshal(&request) - assert.NoError(t, err) + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := evmMocks.NewChain(t) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()). + Return(nil, errors.New("failed to get key")).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) respError := cltest.ParseJSONAPIErrors(t, resp.Body) - require.Equal(t, "failed to get fromAddress: no sending keys available for chain 0", respError.Error()) + require.Equal(t, "failed to get fromAddress: failed to get key", respError.Error()) }) - t.Run("Fails when specified fromAddress is not available for the chain ", func(t *testing.T) { + t.Run("Fails when specified fromAddress is not available for the chain", func(t *testing.T) { + fromAddr := common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371") request := models.CreateEVMTransactionRequest{ - DestinationAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - FromAddress: common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371"), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x", - ChainID: utils.NewBigI(0), + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: fromAddr, + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, } - body, err := json.Marshal(&request) - assert.NoError(t, err) + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := evmMocks.NewChain(t) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() - resp, cleanup := client.Post(txCreatePath, bytes.NewBuffer(body)) - t.Cleanup(cleanup) + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt(), fromAddr).Return(nil, + errors.New("failed to get key for specified whitelist")).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, - "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: no sending "+ - "keys available for chain 0 that match whitelist: [0xfa01fA015c8A5332987319823728982379128371]", + "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: failed to get key for specified whitelist", + respError.Error()) + }) + + newChain := func(t *testing.T, txm txmgr.TxManager, limitDefault uint32) evm.Chain { + chain := evmMocks.NewChain(t) + chain.On("TxManager").Return(txm) + config := evmConfigMocks.NewChainScopedConfig(t) + gasEstimator := evmConfigMocks.NewGasEstimator(t) + gasEstimator.On("LimitDefault").Return(limitDefault) + evmConfig := evmConfigMocks.NewEVM(t) + evmConfig.On("GasEstimator").Return(gasEstimator) + config.On("EVM").Return(evmConfig) + chain.On("Config").Return(config) + return chain + } + t.Run("Populates feeLimit if one is not set", func(t *testing.T) { + payload := []byte("tx_payload") + value := big.NewInt(rand.Int64()) + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + FromAddress: common.HexToAddress("0x39364605296d7c77e7C2089F0e48D527bb309d38"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + hexutils.BytesToHex(payload), + ChainID: chainID, + Value: utils.NewBig(value), + ForwarderAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + } + + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + expectedError := errors.New("failed to create tx") + const feeLimit = uint32(158124) + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: request.FromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: payload, + Value: *value, + FeeLimit: feeLimit, + ForwarderAddress: request.ForwarderAddress, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(txmgr.Tx{}, expectedError).Once() + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm, feeLimit) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt(), request.FromAddress).Return(request.FromAddress, nil).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), respError.Error()) }) } diff --git a/core/web/router.go b/core/web/router.go index fe79f08d673..99101fc6803 100644 --- a/core/web/router.go +++ b/core/web/router.go @@ -279,7 +279,8 @@ func v2Routes(app chainlink.Application, r *gin.RouterGroup) { txs := TransactionsController{app} authv2.GET("/transactions/evm", paginatedRequest(txs.Index)) - authv2.POST("/transactions/evm", txs.Create) + evmTxs := NewEVMTransactionController(app) + authv2.POST("/transactions/evm", evmTxs.Create) authv2.GET("/transactions/evm/:TxHash", txs.Show) authv2.GET("/transactions", paginatedRequest(txs.Index)) authv2.GET("/transactions/:TxHash", txs.Show) From 9958ec33aa575a976f50674839fbd271e5d37ce8 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Fri, 10 Nov 2023 14:27:57 +0100 Subject: [PATCH 03/16] improve tests coverage --- core/web/evm_transactions_controller.go | 27 ++-- core/web/evm_transactions_controller_test.go | 126 +++++++++++++++++-- go.mod | 2 +- 3 files changed, 135 insertions(+), 20 deletions(-) diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 470d5355faa..96d35c766b6 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -3,6 +3,7 @@ package web import ( "database/sql" "fmt" + "math/big" "net/http" "time" @@ -11,6 +12,7 @@ import ( txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" "github.com/smartcontractkit/chainlink/v2/core/chains/evm" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/logger/audit" "github.com/smartcontractkit/chainlink/v2/core/services/chainlink" "github.com/smartcontractkit/chainlink/v2/core/services/keystore" @@ -60,16 +62,20 @@ func (tc *TransactionsController) Show(c *gin.Context) { } type EvmTransactionController struct { - App chainlink.Application - Chains evm.LegacyChainContainer - KeyStore keystore.Eth + Logger logger.SugaredLogger + TxmStorage txmgr.EvmTxStore + AuditLogger audit.AuditLogger + Chains evm.LegacyChainContainer + KeyStore keystore.Eth } func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ - App: app, - Chains: app.GetRelayers().LegacyEVMChains(), - KeyStore: app.GetKeyStore().Eth(), + TxmStorage: app.TxmStorageService(), + Logger: app.GetLogger(), + AuditLogger: app.GetAuditLogger(), + Chains: app.GetRelayers().LegacyEVMChains(), + KeyStore: app.GetKeyStore().Eth(), } } @@ -110,7 +116,7 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { return } - tc.App.GetLogger().Errorf("Failed to get chain", "err", err) + tc.Logger.Errorf("Failed to get chain", "err", err) jsonAPIError(c, http.StatusInternalServerError, err) return } @@ -136,6 +142,9 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { } value := tx.Value.ToInt() + if value == nil { + value = big.NewInt(0) + } etx, err := chain.TxManager().CreateTransaction(c, txmgr.TxRequest{ IdempotencyKey: &tx.IdempotencyKey, FromAddress: tx.FromAddress, @@ -151,7 +160,7 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { return } - tc.App.GetAuditLogger().Audit(audit.EthTransactionCreated, map[string]interface{}{ + tc.AuditLogger.Audit(audit.EthTransactionCreated, map[string]interface{}{ "ethTX": etx, }) @@ -167,7 +176,7 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { } // wait and retrieve tx attempt matching tx ID - attempt, err := FindTxAttempt(c, timeout, etx, tc.App.TxmStorageService().FindTxWithAttempts) + attempt, err := FindTxAttempt(c, timeout, etx, tc.TxmStorage.FindTxWithAttempts) if err != nil { jsonAPIError(c, http.StatusGatewayTimeout, fmt.Errorf("failed to find transaction within timeout: %w", err)) return diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 890563be6b1..29eb600d963 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -25,9 +25,11 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + txmEvmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger/audit" ksMocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" "github.com/smartcontractkit/chainlink/v2/core/store/models" "github.com/smartcontractkit/chainlink/v2/core/utils" @@ -221,7 +223,7 @@ func TestTransactionsController_Create(t *testing.T) { respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, "toAddress must be set", respError.Error()) }) - chainID := utils.NewBigI(rand.Int64()) + chainID := utils.NewBigI(673728) t.Run("Fails if requested chain that is not available", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), @@ -298,18 +300,21 @@ func TestTransactionsController_Create(t *testing.T) { newChain := func(t *testing.T, txm txmgr.TxManager, limitDefault uint32) evm.Chain { chain := evmMocks.NewChain(t) chain.On("TxManager").Return(txm) - config := evmConfigMocks.NewChainScopedConfig(t) + // gas estimator default limit gasEstimator := evmConfigMocks.NewGasEstimator(t) - gasEstimator.On("LimitDefault").Return(limitDefault) + gasEstimator.On("LimitDefault").Return(limitDefault).Maybe() evmConfig := evmConfigMocks.NewEVM(t) - evmConfig.On("GasEstimator").Return(gasEstimator) - config.On("EVM").Return(evmConfig) - chain.On("Config").Return(config) + evmConfig.On("GasEstimator").Return(gasEstimator).Maybe() + config := evmConfigMocks.NewChainScopedConfig(t) + config.On("EVM").Return(evmConfig).Maybe() + chain.On("Config").Return(config).Maybe() + return chain } - t.Run("Populates feeLimit if one is not set", func(t *testing.T) { + t.Run("Correctly populates fields for TxRequest", func(t *testing.T) { payload := []byte("tx_payload") value := big.NewInt(rand.Int64()) + feeLimit := rand.Uint32() request := models.CreateEVMTransactionRequest{ ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), FromAddress: common.HexToAddress("0x39364605296d7c77e7C2089F0e48D527bb309d38"), @@ -318,11 +323,11 @@ func TestTransactionsController_Create(t *testing.T) { ChainID: chainID, Value: utils.NewBig(value), ForwarderAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + FeeLimit: feeLimit, } txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) - expectedError := errors.New("failed to create tx") - const feeLimit = uint32(158124) + expectedError := errors.New("stub error to shortcut execution") txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ IdempotencyKey: &request.IdempotencyKey, FromAddress: request.FromAddress, @@ -335,7 +340,7 @@ func TestTransactionsController_Create(t *testing.T) { }).Return(txmgr.Tx{}, expectedError).Once() chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm, feeLimit) + chain := newChain(t, txm, 0) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() ethKeystore := ksMocks.NewEth(t) @@ -350,4 +355,105 @@ func TestTransactionsController_Create(t *testing.T) { require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), respError.Error()) }) + t.Run("Correctly populates fields for TxRequest with defaults", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + expectedFromAddress := common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2") + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()).Return(expectedFromAddress, nil).Once() + + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + expectedError := errors.New("stub error to shortcut execution") + expectedFeeLimit := rand.Uint32() + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: expectedFromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: []byte{}, + Value: big.Int{}, + FeeLimit: expectedFeeLimit, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(txmgr.Tx{}, expectedError).Once() + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm, expectedFeeLimit) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), + respError.Error()) + }) + t.Run("Happy path", func(t *testing.T) { + payload := []byte("tx_payload") + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + hexutils.BytesToHex(payload), + ChainID: chainID, + Value: utils.NewBigI(6838712), + } + + expectedFromAddress := common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2") + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()).Return(expectedFromAddress, nil).Once() + + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + expectedFeeLimit := uint32(2235235) + + expectedValue := request.Value.ToInt() + tx := txmgr.Tx{ + ID: 54323, + EncodedPayload: payload, + FromAddress: expectedFromAddress, + FeeLimit: expectedFeeLimit, + State: txmgrcommon.TxInProgress, + ToAddress: *request.ToAddress, + Value: *expectedValue, + ChainID: chainID.ToInt(), + } + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: expectedFromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: payload, + Value: *expectedValue, + FeeLimit: expectedFeeLimit, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(tx, nil).Once() + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm, expectedFeeLimit) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + block := int64(56345431) + txWithAttempts := tx + txWithAttempts.TxAttempts = []txmgr.TxAttempt{ + { + Hash: common.HexToHash("0xa1ce83ee556cbcfc6541d5909b0d7f28f6a77399d3bd4340246f684a0f25a7f5"), + BroadcastBeforeBlockNum: &block, + }, + } + + txmStorage := txmEvmMocks.NewEvmTxStore(t) + txmStorage.On("FindTxWithAttempts", tx.ID).Return(txWithAttempts, nil) + + resp := createTx(&web.EvmTransactionController{ + AuditLogger: audit.NoopLogger, + Chains: chainContainer, + KeyStore: ethKeystore, + TxmStorage: txmStorage, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusOK) + }) } diff --git a/go.mod b/go.mod index d61b7b6f61b..425de2f599f 100644 --- a/go.mod +++ b/go.mod @@ -328,7 +328,7 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.15.0 // indirect - github.com/status-im/keycard-go v0.2.0 // indirect + github.com/status-im/keycard-go v0.2.0 github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect From 1ddc0ebb43387b43d000553ce95fd744c8a3c8a3 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Fri, 10 Nov 2023 17:16:55 +0100 Subject: [PATCH 04/16] Flag to enable Txm as service --- .../evm/config/mocks/chain_scoped_config.go | 16 ++++++++++++++++ core/config/app_config.go | 1 + core/config/toml/types.go | 12 ++++++++++++ core/config/txm_as_service.go | 5 +++++ core/services/chainlink/config_general.go | 4 ++++ core/services/chainlink/config_test.go | 1 + core/services/chainlink/config_txm_as_service.go | 16 ++++++++++++++++ .../testdata/config-empty-effective.toml | 3 +++ .../services/chainlink/testdata/config-full.toml | 3 +++ .../testdata/config-multi-chain-effective.toml | 3 +++ core/web/evm_transactions_controller.go | 6 ++++++ core/web/evm_transactions_controller_test.go | 14 ++++++++++++++ 12 files changed, 84 insertions(+) create mode 100644 core/config/txm_as_service.go create mode 100644 core/services/chainlink/config_txm_as_service.go diff --git a/core/chains/evm/config/mocks/chain_scoped_config.go b/core/chains/evm/config/mocks/chain_scoped_config.go index cb18282f495..64c3530d66a 100644 --- a/core/chains/evm/config/mocks/chain_scoped_config.go +++ b/core/chains/evm/config/mocks/chain_scoped_config.go @@ -513,6 +513,22 @@ func (_m *ChainScopedConfig) Tracing() coreconfig.Tracing { return r0 } +// TxmAsService provides a mock function with given fields: +func (_m *ChainScopedConfig) TxmAsService() coreconfig.TxmAsService { + ret := _m.Called() + + var r0 coreconfig.TxmAsService + if rf, ok := ret.Get(0).(func() coreconfig.TxmAsService); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(coreconfig.TxmAsService) + } + } + + return r0 +} + // Validate provides a mock function with given fields: func (_m *ChainScopedConfig) Validate() error { ret := _m.Called() diff --git a/core/config/app_config.go b/core/config/app_config.go index 648939b871b..862376b47e8 100644 --- a/core/config/app_config.go +++ b/core/config/app_config.go @@ -54,6 +54,7 @@ type AppConfig interface { Threshold() Threshold WebServer() WebServer Tracing() Tracing + TxmAsService() TxmAsService } type DatabaseBackupMode string diff --git a/core/config/toml/types.go b/core/config/toml/types.go index 61962d43e5f..d6c1990a5cd 100644 --- a/core/config/toml/types.go +++ b/core/config/toml/types.go @@ -54,6 +54,7 @@ type Core struct { Sentry Sentry `toml:",omitempty"` Insecure Insecure `toml:",omitempty"` Tracing Tracing `toml:",omitempty"` + TxmAsService TxmAsService `toml:",omitempty"` } // SetFrom updates c with any non-nil values from f. (currently TOML field only!) @@ -88,6 +89,7 @@ func (c *Core) SetFrom(f *Core) { c.Sentry.setFrom(&f.Sentry) c.Insecure.setFrom(&f.Insecure) c.Tracing.setFrom(&f.Tracing) + c.TxmAsService.setFrom(&f.TxmAsService) } func (c *Core) ValidateConfig() (err error) { @@ -1530,3 +1532,13 @@ func isValidURI(uri string) bool { func isValidHostname(hostname string) bool { return hostnameRegex.MatchString(hostname) } + +type TxmAsService struct { + Enabled *bool +} + +func (t *TxmAsService) setFrom(f *TxmAsService) { + if v := f.Enabled; v != nil { + t.Enabled = f.Enabled + } +} diff --git a/core/config/txm_as_service.go b/core/config/txm_as_service.go new file mode 100644 index 00000000000..1672a66e482 --- /dev/null +++ b/core/config/txm_as_service.go @@ -0,0 +1,5 @@ +package config + +type TxmAsService interface { + Enabled() bool +} diff --git a/core/services/chainlink/config_general.go b/core/services/chainlink/config_general.go index 6a835e09c89..d8aabefbd03 100644 --- a/core/services/chainlink/config_general.go +++ b/core/services/chainlink/config_general.go @@ -518,4 +518,8 @@ func (g *generalConfig) Tracing() coreconfig.Tracing { return &tracingConfig{s: g.c.Tracing} } +func (g *generalConfig) TxmAsService() config.TxmAsService { + return &txmAsServiceConfig{c: g.c.TxmAsService} +} + var zeroSha256Hash = models.Sha256Hash{} diff --git a/core/services/chainlink/config_test.go b/core/services/chainlink/config_test.go index cc3fda167de..63c95ae9289 100644 --- a/core/services/chainlink/config_test.go +++ b/core/services/chainlink/config_test.go @@ -665,6 +665,7 @@ func TestConfig_Marshal(t *testing.T) { }, }, } + full.TxmAsService = toml.TxmAsService{Enabled: ptr(true)} for _, tt := range []struct { name string diff --git a/core/services/chainlink/config_txm_as_service.go b/core/services/chainlink/config_txm_as_service.go new file mode 100644 index 00000000000..50e1fe381d9 --- /dev/null +++ b/core/services/chainlink/config_txm_as_service.go @@ -0,0 +1,16 @@ +package chainlink + +import ( + "github.com/smartcontractkit/chainlink/v2/core/config" + "github.com/smartcontractkit/chainlink/v2/core/config/toml" +) + +var _ config.TxmAsService = (*txmAsServiceConfig)(nil) + +type txmAsServiceConfig struct { + c toml.TxmAsService +} + +func (t *txmAsServiceConfig) Enabled() bool { + return t.c.Enabled != nil && *t.c.Enabled +} diff --git a/core/services/chainlink/testdata/config-empty-effective.toml b/core/services/chainlink/testdata/config-empty-effective.toml index f5d775fe744..55edfd0cf01 100644 --- a/core/services/chainlink/testdata/config-empty-effective.toml +++ b/core/services/chainlink/testdata/config-empty-effective.toml @@ -232,3 +232,6 @@ Enabled = false CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 + +[TxmAsService] +Enabled = false diff --git a/core/services/chainlink/testdata/config-full.toml b/core/services/chainlink/testdata/config-full.toml index 5ede10ef695..7053153e0bc 100644 --- a/core/services/chainlink/testdata/config-full.toml +++ b/core/services/chainlink/testdata/config-full.toml @@ -243,6 +243,9 @@ SamplingRatio = 1.0 env = 'dev' test = 'load' +[TxmAsService] +Enabled = true + [[EVM]] ChainID = '1' Enabled = false diff --git a/core/services/chainlink/testdata/config-multi-chain-effective.toml b/core/services/chainlink/testdata/config-multi-chain-effective.toml index 9dd0be8f5d2..853e393d58c 100644 --- a/core/services/chainlink/testdata/config-multi-chain-effective.toml +++ b/core/services/chainlink/testdata/config-multi-chain-effective.toml @@ -233,6 +233,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 96d35c766b6..268bcdfd23b 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -62,6 +62,7 @@ func (tc *TransactionsController) Show(c *gin.Context) { } type EvmTransactionController struct { + Enabled bool Logger logger.SugaredLogger TxmStorage txmgr.EvmTxStore AuditLogger audit.AuditLogger @@ -71,6 +72,7 @@ type EvmTransactionController struct { func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ + Enabled: app.GetConfig().TxmAsService().Enabled(), TxmStorage: app.TxmStorageService(), Logger: app.GetLogger(), AuditLogger: app.GetAuditLogger(), @@ -82,6 +84,10 @@ func NewEVMTransactionController(app chainlink.Application) *EvmTransactionContr // Create signs and sends transaction from specified address. If address is not provided uses one of enabled keys for // specified chain. func (tc *EvmTransactionController) Create(c *gin.Context) { + if !tc.Enabled { + jsonAPIError(c, http.StatusUnprocessableEntity, errors.New("transactions creation disabled. To enable set TxmAsService.Enabled=true")) + return + } var tx models.CreateEVMTransactionRequest if err := c.ShouldBindJSON(&tx); err != nil { jsonAPIError(c, http.StatusBadRequest, err) diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 29eb600d963..9884cc2c805 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -152,6 +152,19 @@ func TestTransactionsController_Show_NotFound(t *testing.T) { func TestTransactionsController_Create(t *testing.T) { t.Parallel() const txCreatePath = "/v2/transactions/evm" + t.Run("Returns error if endpoint is disabled", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodPost, txCreatePath, nil) + router := gin.New() + controller := &web.EvmTransactionController{ + Enabled: false, + } + router.POST(txCreatePath, controller.Create) + resp := httptest.NewRecorder() + router.ServeHTTP(resp, req) + cltest.AssertServerResponse(t, resp.Result(), http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "transactions creation disabled. To enable set TxmAsService.Enabled=true", respError.Error()) + }) createTx := func(controller *web.EvmTransactionController, request interface{}) *httptest.ResponseRecorder { body, err := json.Marshal(&request) @@ -160,6 +173,7 @@ func TestTransactionsController_Create(t *testing.T) { w := httptest.NewRecorder() req, _ := http.NewRequest(http.MethodPost, txCreatePath, bytes.NewBuffer(body)) router := gin.New() + controller.Enabled = true router.POST(txCreatePath, controller.Create) router.ServeHTTP(w, req) return w From d587ff405c11dcc5dcca601996949f52b023fed8 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Fri, 10 Nov 2023 17:24:22 +0100 Subject: [PATCH 05/16] require admin role to send tx --- core/web/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/web/router.go b/core/web/router.go index 99101fc6803..0ddb7251ab9 100644 --- a/core/web/router.go +++ b/core/web/router.go @@ -280,7 +280,7 @@ func v2Routes(app chainlink.Application, r *gin.RouterGroup) { txs := TransactionsController{app} authv2.GET("/transactions/evm", paginatedRequest(txs.Index)) evmTxs := NewEVMTransactionController(app) - authv2.POST("/transactions/evm", evmTxs.Create) + authv2.POST("/transactions/evm", auth.RequiresAdminRole(evmTxs.Create)) authv2.GET("/transactions/evm/:TxHash", txs.Show) authv2.GET("/transactions", paginatedRequest(txs.Index)) authv2.GET("/transactions/:TxHash", txs.Show) From 80c4fb7e370cb83e5f375893de73f598baed7074 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Fri, 10 Nov 2023 17:27:03 +0100 Subject: [PATCH 06/16] moved question into PR --- core/web/evm_transactions_controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 268bcdfd23b..da3274f7d8e 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -143,7 +143,6 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { } if tx.FeeLimit == 0 { - // TODO: is it a right place to get default limit? tx.FeeLimit = chain.Config().EVM().GasEstimator().LimitDefault() } From 565c9c3023259480162a6a5207001e802bf1feae Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Fri, 10 Nov 2023 18:39:50 +0100 Subject: [PATCH 07/16] fix config docs --- core/config/docs/core.toml | 5 +++++ core/services/chainlink/mocks/general_config.go | 16 ++++++++++++++++ .../testdata/config-empty-effective.toml | 3 +++ core/web/resolver/testdata/config-full.toml | 3 +++ .../testdata/config-multi-chain-effective.toml | 3 +++ docs/CONFIG.md | 14 ++++++++++++++ testdata/scripts/node/validate/default.txtar | 3 +++ .../validate/disk-based-logging-disabled.txtar | 3 +++ .../validate/disk-based-logging-no-dir.txtar | 3 +++ .../node/validate/disk-based-logging.txtar | 3 +++ testdata/scripts/node/validate/invalid.txtar | 3 +++ testdata/scripts/node/validate/valid.txtar | 3 +++ testdata/scripts/node/validate/warnings.txtar | 3 +++ 13 files changed, 65 insertions(+) diff --git a/core/config/docs/core.toml b/core/config/docs/core.toml index 0a8e6aba3be..c2fc2560fb1 100644 --- a/core/config/docs/core.toml +++ b/core/config/docs/core.toml @@ -597,3 +597,8 @@ SamplingRatio = 1.0 # Example [Tracing.Attributes] # env is an example user specified key-value pair env = "test" # Example + +[TxmAsService] +# Enabled turns Transaction Manager as a service feature on or off. When enabled exposes endpoint to submit arbitrary EVM transaction +# using one of the keys enabled for the specified chain. +Enabled = false # Default diff --git a/core/services/chainlink/mocks/general_config.go b/core/services/chainlink/mocks/general_config.go index 98796e90053..4592cc218b6 100644 --- a/core/services/chainlink/mocks/general_config.go +++ b/core/services/chainlink/mocks/general_config.go @@ -591,6 +591,22 @@ func (_m *GeneralConfig) Tracing() config.Tracing { return r0 } +// TxmAsService provides a mock function with given fields: +func (_m *GeneralConfig) TxmAsService() config.TxmAsService { + ret := _m.Called() + + var r0 config.TxmAsService + if rf, ok := ret.Get(0).(func() config.TxmAsService); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(config.TxmAsService) + } + } + + return r0 +} + // Validate provides a mock function with given fields: func (_m *GeneralConfig) Validate() error { ret := _m.Called() diff --git a/core/web/resolver/testdata/config-empty-effective.toml b/core/web/resolver/testdata/config-empty-effective.toml index f5d775fe744..55edfd0cf01 100644 --- a/core/web/resolver/testdata/config-empty-effective.toml +++ b/core/web/resolver/testdata/config-empty-effective.toml @@ -232,3 +232,6 @@ Enabled = false CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 + +[TxmAsService] +Enabled = false diff --git a/core/web/resolver/testdata/config-full.toml b/core/web/resolver/testdata/config-full.toml index 95d898c353b..3d5adbbc049 100644 --- a/core/web/resolver/testdata/config-full.toml +++ b/core/web/resolver/testdata/config-full.toml @@ -243,6 +243,9 @@ SamplingRatio = 1.0 env = 'dev' test = 'load' +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' Enabled = false diff --git a/core/web/resolver/testdata/config-multi-chain-effective.toml b/core/web/resolver/testdata/config-multi-chain-effective.toml index 9dd0be8f5d2..853e393d58c 100644 --- a/core/web/resolver/testdata/config-multi-chain-effective.toml +++ b/core/web/resolver/testdata/config-multi-chain-effective.toml @@ -233,6 +233,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 1eb9cd5023d..55e4c84348a 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -1638,6 +1638,20 @@ env = "test" # Example ``` env is an example user specified key-value pair +## TxmAsService +```toml +[TxmAsService] +Enabled = false # Default +``` + + +### Enabled +```toml +Enabled = false # Default +``` +Enabled turns Transaction Manager as a service feature on or off. When enabled exposes endpoint to submit arbitrary EVM transaction +using one of the keys enabled for the specified chain. + ## EVM EVM defaults depend on ChainID: diff --git a/testdata/scripts/node/validate/default.txtar b/testdata/scripts/node/validate/default.txtar index 8a3b1af96fa..d7ef0ac7bbb 100644 --- a/testdata/scripts/node/validate/default.txtar +++ b/testdata/scripts/node/validate/default.txtar @@ -245,6 +245,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + Invalid configuration: invalid secrets: 2 errors: - Database.URL: empty: must be provided and non-empty - Password.Keystore: empty: must be provided and non-empty diff --git a/testdata/scripts/node/validate/disk-based-logging-disabled.txtar b/testdata/scripts/node/validate/disk-based-logging-disabled.txtar index 31fded1b423..0dd9c2e8a17 100644 --- a/testdata/scripts/node/validate/disk-based-logging-disabled.txtar +++ b/testdata/scripts/node/validate/disk-based-logging-disabled.txtar @@ -289,6 +289,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar b/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar index 78fc976912c..3939df994e5 100644 --- a/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar +++ b/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar @@ -289,6 +289,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/disk-based-logging.txtar b/testdata/scripts/node/validate/disk-based-logging.txtar index 226a7bbb3b4..bf7b1e0c412 100644 --- a/testdata/scripts/node/validate/disk-based-logging.txtar +++ b/testdata/scripts/node/validate/disk-based-logging.txtar @@ -289,6 +289,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/invalid.txtar b/testdata/scripts/node/validate/invalid.txtar index 5cd3d567467..389f98a60b5 100644 --- a/testdata/scripts/node/validate/invalid.txtar +++ b/testdata/scripts/node/validate/invalid.txtar @@ -279,6 +279,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/valid.txtar b/testdata/scripts/node/validate/valid.txtar index fd24150b587..d1f3230be8b 100644 --- a/testdata/scripts/node/validate/valid.txtar +++ b/testdata/scripts/node/validate/valid.txtar @@ -286,6 +286,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/warnings.txtar b/testdata/scripts/node/validate/warnings.txtar index 828d953da9a..8fa48027607 100644 --- a/testdata/scripts/node/validate/warnings.txtar +++ b/testdata/scripts/node/validate/warnings.txtar @@ -282,6 +282,9 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 +[TxmAsService] +Enabled = false + # Configuration warning: 2 errors: - P2P.V1: is deprecated and will be removed in a future version From e2dde3bb0e90d3c97eb8ffad3055d6a56443ab93 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Tue, 14 Nov 2023 13:45:27 +0100 Subject: [PATCH 08/16] address comments --- core/store/models/common.go | 18 +++++++------- core/web/evm_transactions_controller.go | 25 ++------------------ core/web/evm_transactions_controller_test.go | 22 ++++------------- 3 files changed, 14 insertions(+), 51 deletions(-) diff --git a/core/store/models/common.go b/core/store/models/common.go index 0b35062dc94..e13d79c7059 100644 --- a/core/store/models/common.go +++ b/core/store/models/common.go @@ -665,14 +665,12 @@ func (h ServiceHeader) Validate() (err error) { // CreateEVMTransactionRequest represents a request to create request to submit evm transaction. type CreateEVMTransactionRequest struct { - IdempotencyKey string `json:"idempotencyKey"` - ChainID *utils.Big `json:"chainID"` - ToAddress *common.Address `json:"toAddress"` - FromAddress common.Address `json:"fromAddress"` // optional, if not set we use one of accounts available for specified chain - EncodedPayload string `json:"encodedPayload"` // hex encoded payload - Value *utils.Big `json:"value"` - ForwarderAddress common.Address `json:"forwarderAddress"` - FeeLimit uint32 `json:"feeLimit"` - SkipWaitTxAttempt bool `json:"skipWaitTxAttempt"` - WaitAttemptTimeout *time.Duration `json:"waitAttemptTimeout"` + IdempotencyKey string `json:"idempotencyKey"` + ChainID *utils.Big `json:"chainID"` + ToAddress *common.Address `json:"toAddress"` + FromAddress common.Address `json:"fromAddress"` // optional, if not set we use one of accounts available for specified chain + EncodedPayload string `json:"encodedPayload"` // hex encoded payload + Value *utils.Big `json:"value"` + ForwarderAddress common.Address `json:"forwarderAddress"` + FeeLimit uint32 `json:"feeLimit"` } diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index da3274f7d8e..9c7ca4c53be 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -2,10 +2,8 @@ package web import ( "database/sql" - "fmt" "math/big" "net/http" - "time" "github.com/ethereum/go-ethereum/common/hexutil" @@ -64,7 +62,6 @@ func (tc *TransactionsController) Show(c *gin.Context) { type EvmTransactionController struct { Enabled bool Logger logger.SugaredLogger - TxmStorage txmgr.EvmTxStore AuditLogger audit.AuditLogger Chains evm.LegacyChainContainer KeyStore keystore.Eth @@ -73,7 +70,6 @@ type EvmTransactionController struct { func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ Enabled: app.GetConfig().TxmAsService().Enabled(), - TxmStorage: app.TxmStorageService(), Logger: app.GetLogger(), AuditLogger: app.GetAuditLogger(), Chains: app.GetRelayers().LegacyEVMChains(), @@ -134,7 +130,7 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { return } } else { - _, err = tc.KeyStore.GetRoundRobinAddress(tx.ChainID.ToInt(), tx.FromAddress) + err = tc.KeyStore.CheckEnabled(tx.FromAddress, tx.ChainID.ToInt()) if err != nil { jsonAPIError(c, http.StatusUnprocessableEntity, errors.Errorf("fromAddress %v is not available: %v", tx.FromAddress, err)) @@ -169,22 +165,5 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { "ethTX": etx, }) - // skip waiting for txmgr to create TxAttempt - if tx.SkipWaitTxAttempt { - jsonAPIResponse(c, presenters.NewEthTxResource(etx), "eth_tx") - return - } - - timeout := 10 * time.Second // default - if tx.WaitAttemptTimeout != nil { - timeout = *tx.WaitAttemptTimeout - } - - // wait and retrieve tx attempt matching tx ID - attempt, err := FindTxAttempt(c, timeout, etx, tc.TxmStorage.FindTxWithAttempts) - if err != nil { - jsonAPIError(c, http.StatusGatewayTimeout, fmt.Errorf("failed to find transaction within timeout: %w", err)) - return - } - jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(attempt), "eth_tx") + jsonAPIResponse(c, presenters.NewEthTxResource(etx), "eth_tx") } diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 9884cc2c805..add35e25dd9 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -25,7 +25,6 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" - txmEvmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" @@ -297,8 +296,8 @@ func TestTransactionsController_Create(t *testing.T) { chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() ethKeystore := ksMocks.NewEth(t) - ethKeystore.On("GetRoundRobinAddress", chainID.ToInt(), fromAddr).Return(nil, - errors.New("failed to get key for specified whitelist")).Once() + ethKeystore.On("CheckEnabled", fromAddr, chainID.ToInt()). + Return(errors.New("no eth key exists with address")).Once() resp := createTx(&web.EvmTransactionController{ Chains: chainContainer, KeyStore: ethKeystore, @@ -307,7 +306,7 @@ func TestTransactionsController_Create(t *testing.T) { cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, - "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: failed to get key for specified whitelist", + "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: no eth key exists with address", respError.Error()) }) @@ -358,7 +357,7 @@ func TestTransactionsController_Create(t *testing.T) { chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() ethKeystore := ksMocks.NewEth(t) - ethKeystore.On("GetRoundRobinAddress", chainID.ToInt(), request.FromAddress).Return(request.FromAddress, nil).Once() + ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() resp := createTx(&web.EvmTransactionController{ Chains: chainContainer, KeyStore: ethKeystore, @@ -449,23 +448,10 @@ func TestTransactionsController_Create(t *testing.T) { chainContainer := evmMocks.NewLegacyChainContainer(t) chain := newChain(t, txm, expectedFeeLimit) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() - block := int64(56345431) - txWithAttempts := tx - txWithAttempts.TxAttempts = []txmgr.TxAttempt{ - { - Hash: common.HexToHash("0xa1ce83ee556cbcfc6541d5909b0d7f28f6a77399d3bd4340246f684a0f25a7f5"), - BroadcastBeforeBlockNum: &block, - }, - } - - txmStorage := txmEvmMocks.NewEvmTxStore(t) - txmStorage.On("FindTxWithAttempts", tx.ID).Return(txWithAttempts, nil) - resp := createTx(&web.EvmTransactionController{ AuditLogger: audit.NoopLogger, Chains: chainContainer, KeyStore: ethKeystore, - TxmStorage: txmStorage, }, request).Result() cltest.AssertServerResponse(t, resp, http.StatusOK) From c29f27d6cea2c148be0cab942815b957ffddd728 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 13:20:56 +0100 Subject: [PATCH 09/16] remove redundant dependency --- core/web/evm_transactions_controller_test.go | 6 +++--- go.mod | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index add35e25dd9..727c021fc1a 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -13,7 +13,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/gin-gonic/gin" "github.com/pkg/errors" - "github.com/status-im/keycard-go/hexutils" "github.com/stretchr/testify/mock" txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" @@ -328,11 +327,12 @@ func TestTransactionsController_Create(t *testing.T) { payload := []byte("tx_payload") value := big.NewInt(rand.Int64()) feeLimit := rand.Uint32() + request := models.CreateEVMTransactionRequest{ ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), FromAddress: common.HexToAddress("0x39364605296d7c77e7C2089F0e48D527bb309d38"), IdempotencyKey: "idempotency_key", - EncodedPayload: "0x" + hexutils.BytesToHex(payload), + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), ChainID: chainID, Value: utils.NewBig(value), ForwarderAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), @@ -412,7 +412,7 @@ func TestTransactionsController_Create(t *testing.T) { request := models.CreateEVMTransactionRequest{ ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), IdempotencyKey: "idempotency_key", - EncodedPayload: "0x" + hexutils.BytesToHex(payload), + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), ChainID: chainID, Value: utils.NewBigI(6838712), } diff --git a/go.mod b/go.mod index 8d27c758f11..0a85fe7f488 100644 --- a/go.mod +++ b/go.mod @@ -327,7 +327,7 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.15.0 // indirect - github.com/status-im/keycard-go v0.2.0 + github.com/status-im/keycard-go v0.2.0 // indirect github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect From 1dd243080d3e16f04fe18cf8e3201fa4c14d5d4a Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 13:53:03 +0100 Subject: [PATCH 10/16] proper status code on tx failure --- core/web/evm_transactions_controller.go | 2 +- core/web/evm_transactions_controller_test.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 9c7ca4c53be..92d316acd7a 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -157,7 +157,7 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { Strategy: txmgrcommon.NewSendEveryStrategy(), }) if err != nil { - jsonAPIError(c, http.StatusBadRequest, errors.Errorf("transaction failed: %v", err)) + jsonAPIError(c, http.StatusInternalServerError, errors.Errorf("transaction failed: %v", err)) return } diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 727c021fc1a..8e949b8c79c 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -363,7 +363,7 @@ func TestTransactionsController_Create(t *testing.T) { KeyStore: ethKeystore, }, request).Result() - cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + cltest.AssertServerResponse(t, resp, http.StatusInternalServerError) respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), respError.Error()) @@ -402,7 +402,8 @@ func TestTransactionsController_Create(t *testing.T) { KeyStore: ethKeystore, }, request).Result() - cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + // we do not really care about results here as main check is during CreateTransaction call + cltest.AssertServerResponse(t, resp, http.StatusInternalServerError) respError := cltest.ParseJSONAPIErrors(t, resp.Body) require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), respError.Error()) From 94a3c0a42ba976635bf85f2f7865d54d94409ba8 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 14:17:51 +0100 Subject: [PATCH 11/16] wait for tx attempt to return properly populated resource --- core/web/evm_transactions_controller.go | 39 +++++-- core/web/evm_transactions_controller_test.go | 108 ++++++++++++++----- 2 files changed, 110 insertions(+), 37 deletions(-) diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index 92d316acd7a..bde170f7781 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -1,9 +1,11 @@ package web import ( + "context" "database/sql" "math/big" "net/http" + "time" "github.com/ethereum/go-ethereum/common/hexutil" @@ -60,20 +62,24 @@ func (tc *TransactionsController) Show(c *gin.Context) { } type EvmTransactionController struct { - Enabled bool - Logger logger.SugaredLogger - AuditLogger audit.AuditLogger - Chains evm.LegacyChainContainer - KeyStore keystore.Eth + Enabled bool + Logger logger.SugaredLogger + TxmStore txmgr.EvmTxStore + AuditLogger audit.AuditLogger + Chains evm.LegacyChainContainer + KeyStore keystore.Eth + TxAttemptWaitDur time.Duration } func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ - Enabled: app.GetConfig().TxmAsService().Enabled(), - Logger: app.GetLogger(), - AuditLogger: app.GetAuditLogger(), - Chains: app.GetRelayers().LegacyEVMChains(), - KeyStore: app.GetKeyStore().Eth(), + Enabled: app.GetConfig().TxmAsService().Enabled(), + TxmStore: app.TxmStorageService(), + Logger: app.GetLogger(), + AuditLogger: app.GetAuditLogger(), + Chains: app.GetRelayers().LegacyEVMChains(), + KeyStore: app.GetKeyStore().Eth(), + TxAttemptWaitDur: 10 * time.Second, } } @@ -165,5 +171,16 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { "ethTX": etx, }) - jsonAPIResponse(c, presenters.NewEthTxResource(etx), "eth_tx") + // wait and retrieve tx attempt matching tx ID + attempt, err := FindTxAttempt(c, tc.TxAttemptWaitDur, etx, tc.TxmStore.FindTxWithAttempts) + if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + jsonAPIError(c, http.StatusGatewayTimeout, errors.Errorf("failed to find transaction within timeout: %v", err)) + return + } + + jsonAPIError(c, http.StatusInternalServerError, errors.Errorf("failed to find transaction: %v", err)) + return + } + jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(attempt), "eth_tx") } diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 8e949b8c79c..9f679f4a1c3 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -24,6 +24,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + txmEvmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" @@ -408,51 +409,106 @@ func TestTransactionsController_Create(t *testing.T) { require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), respError.Error()) }) - t.Run("Happy path", func(t *testing.T) { - payload := []byte("tx_payload") - request := models.CreateEVMTransactionRequest{ - ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x" + fmt.Sprintf("%X", payload), - ChainID: chainID, - Value: utils.NewBigI(6838712), - } - - expectedFromAddress := common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2") - ethKeystore := ksMocks.NewEth(t) - ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()).Return(expectedFromAddress, nil).Once() - - txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) - expectedFeeLimit := uint32(2235235) - expectedValue := request.Value.ToInt() - tx := txmgr.Tx{ - ID: 54323, + payload := []byte("tx_payload") + expectedFeeLimit := uint32(2235235) + const txID = int64(54323) + newTxFromRequest := func(request models.CreateEVMTransactionRequest) txmgr.Tx { + return txmgr.Tx{ + ID: txID, EncodedPayload: payload, - FromAddress: expectedFromAddress, + FromAddress: request.FromAddress, FeeLimit: expectedFeeLimit, State: txmgrcommon.TxInProgress, ToAddress: *request.ToAddress, - Value: *expectedValue, + Value: *request.Value.ToInt(), ChainID: chainID.ToInt(), } + } + newTxManager := func(request models.CreateEVMTransactionRequest) txmgr.TxManager { + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + tx := newTxFromRequest(request) txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ IdempotencyKey: &request.IdempotencyKey, - FromAddress: expectedFromAddress, + FromAddress: request.FromAddress, ToAddress: *request.ToAddress, EncodedPayload: payload, - Value: *expectedValue, + Value: *request.Value.ToInt(), FeeLimit: expectedFeeLimit, Strategy: txmgrcommon.NewSendEveryStrategy(), }).Return(tx, nil).Once() + return txm + } + t.Run("Fails to find tx attempt in time", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + FromAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), + ChainID: chainID, + Value: utils.NewBigI(6838712), + } + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() + txm := newTxManager(request) + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm, expectedFeeLimit) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + txmStorage := txmEvmMocks.NewEvmTxStore(t) + txmStorage.On("FindTxWithAttempts", txID).Return(txmgr.Tx{}, nil).Maybe() + + resp := createTx(&web.EvmTransactionController{ + AuditLogger: audit.NoopLogger, + Chains: chainContainer, + KeyStore: ethKeystore, + TxmStore: txmStorage, + TxAttemptWaitDur: testutils.TestInterval, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusGatewayTimeout) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "failed to find transaction within timeout: context deadline exceeded - tx may still have been broadcast", + respError.Error()) + }) + t.Run("Happy path", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + FromAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), + ChainID: chainID, + Value: utils.NewBigI(6838712), + } + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() + + txm := newTxManager(request) chainContainer := evmMocks.NewLegacyChainContainer(t) chain := newChain(t, txm, expectedFeeLimit) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + block := int64(56345431) + txWithAttempts := newTxFromRequest(request) + txWithAttempts.TxAttempts = []txmgr.TxAttempt{ + { + Hash: common.HexToHash("0xa1ce83ee556cbcfc6541d5909b0d7f28f6a77399d3bd4340246f684a0f25a7f5"), + BroadcastBeforeBlockNum: &block, + }, + } + + txmStorage := txmEvmMocks.NewEvmTxStore(t) + txmStorage.On("FindTxWithAttempts", txWithAttempts.ID).Return(txWithAttempts, nil) + resp := createTx(&web.EvmTransactionController{ - AuditLogger: audit.NoopLogger, - Chains: chainContainer, - KeyStore: ethKeystore, + AuditLogger: audit.NoopLogger, + Chains: chainContainer, + KeyStore: ethKeystore, + TxmStore: txmStorage, + TxAttemptWaitDur: testutils.DefaultWaitTimeout, }, request).Result() cltest.AssertServerResponse(t, resp, http.StatusOK) From 4c23c1618a52eb2a8a8bf6dafecf6def91b50ee2 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 15:23:09 +0100 Subject: [PATCH 12/16] remove evm config mock generation --- core/chains/evm/config/config.go | 1 - core/chains/evm/config/mocks/evm.go | 456 ------------------- core/web/evm_transactions_controller_test.go | 32 +- 3 files changed, 14 insertions(+), 475 deletions(-) delete mode 100644 core/chains/evm/config/mocks/evm.go diff --git a/core/chains/evm/config/config.go b/core/chains/evm/config/config.go index ac13db26886..f8ec030969e 100644 --- a/core/chains/evm/config/config.go +++ b/core/chains/evm/config/config.go @@ -10,7 +10,6 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/config" ) -//go:generate mockery --quiet --name EVM --output ./mocks/ --case=underscore type EVM interface { HeadTracker() HeadTracker BalanceMonitor() BalanceMonitor diff --git a/core/chains/evm/config/mocks/evm.go b/core/chains/evm/config/mocks/evm.go deleted file mode 100644 index 746ee8cbe51..00000000000 --- a/core/chains/evm/config/mocks/evm.go +++ /dev/null @@ -1,456 +0,0 @@ -// Code generated by mockery v2.35.4. DO NOT EDIT. - -package mocks - -import ( - big "math/big" - - assets "github.com/smartcontractkit/chainlink/v2/core/assets" - - config "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" - - coreconfig "github.com/smartcontractkit/chainlink/v2/core/config" - - mock "github.com/stretchr/testify/mock" - - time "time" -) - -// EVM is an autogenerated mock type for the EVM type -type EVM struct { - mock.Mock -} - -// AutoCreateKey provides a mock function with given fields: -func (_m *EVM) AutoCreateKey() bool { - ret := _m.Called() - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// BalanceMonitor provides a mock function with given fields: -func (_m *EVM) BalanceMonitor() config.BalanceMonitor { - ret := _m.Called() - - var r0 config.BalanceMonitor - if rf, ok := ret.Get(0).(func() config.BalanceMonitor); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.BalanceMonitor) - } - } - - return r0 -} - -// BlockBackfillDepth provides a mock function with given fields: -func (_m *EVM) BlockBackfillDepth() uint64 { - ret := _m.Called() - - var r0 uint64 - if rf, ok := ret.Get(0).(func() uint64); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint64) - } - - return r0 -} - -// BlockBackfillSkip provides a mock function with given fields: -func (_m *EVM) BlockBackfillSkip() bool { - ret := _m.Called() - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// BlockEmissionIdleWarningThreshold provides a mock function with given fields: -func (_m *EVM) BlockEmissionIdleWarningThreshold() time.Duration { - ret := _m.Called() - - var r0 time.Duration - if rf, ok := ret.Get(0).(func() time.Duration); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(time.Duration) - } - - return r0 -} - -// ChainID provides a mock function with given fields: -func (_m *EVM) ChainID() *big.Int { - ret := _m.Called() - - var r0 *big.Int - if rf, ok := ret.Get(0).(func() *big.Int); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*big.Int) - } - } - - return r0 -} - -// ChainType provides a mock function with given fields: -func (_m *EVM) ChainType() coreconfig.ChainType { - ret := _m.Called() - - var r0 coreconfig.ChainType - if rf, ok := ret.Get(0).(func() coreconfig.ChainType); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(coreconfig.ChainType) - } - - return r0 -} - -// FinalityDepth provides a mock function with given fields: -func (_m *EVM) FinalityDepth() uint32 { - ret := _m.Called() - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// FinalityTagEnabled provides a mock function with given fields: -func (_m *EVM) FinalityTagEnabled() bool { - ret := _m.Called() - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// FlagsContractAddress provides a mock function with given fields: -func (_m *EVM) FlagsContractAddress() string { - ret := _m.Called() - - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(string) - } - - return r0 -} - -// GasEstimator provides a mock function with given fields: -func (_m *EVM) GasEstimator() config.GasEstimator { - ret := _m.Called() - - var r0 config.GasEstimator - if rf, ok := ret.Get(0).(func() config.GasEstimator); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.GasEstimator) - } - } - - return r0 -} - -// HeadTracker provides a mock function with given fields: -func (_m *EVM) HeadTracker() config.HeadTracker { - ret := _m.Called() - - var r0 config.HeadTracker - if rf, ok := ret.Get(0).(func() config.HeadTracker); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.HeadTracker) - } - } - - return r0 -} - -// IsEnabled provides a mock function with given fields: -func (_m *EVM) IsEnabled() bool { - ret := _m.Called() - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// LinkContractAddress provides a mock function with given fields: -func (_m *EVM) LinkContractAddress() string { - ret := _m.Called() - - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(string) - } - - return r0 -} - -// LogBackfillBatchSize provides a mock function with given fields: -func (_m *EVM) LogBackfillBatchSize() uint32 { - ret := _m.Called() - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// LogKeepBlocksDepth provides a mock function with given fields: -func (_m *EVM) LogKeepBlocksDepth() uint32 { - ret := _m.Called() - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// LogPollInterval provides a mock function with given fields: -func (_m *EVM) LogPollInterval() time.Duration { - ret := _m.Called() - - var r0 time.Duration - if rf, ok := ret.Get(0).(func() time.Duration); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(time.Duration) - } - - return r0 -} - -// MinContractPayment provides a mock function with given fields: -func (_m *EVM) MinContractPayment() *assets.Link { - ret := _m.Called() - - var r0 *assets.Link - if rf, ok := ret.Get(0).(func() *assets.Link); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*assets.Link) - } - } - - return r0 -} - -// MinIncomingConfirmations provides a mock function with given fields: -func (_m *EVM) MinIncomingConfirmations() uint32 { - ret := _m.Called() - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// NodeNoNewHeadsThreshold provides a mock function with given fields: -func (_m *EVM) NodeNoNewHeadsThreshold() time.Duration { - ret := _m.Called() - - var r0 time.Duration - if rf, ok := ret.Get(0).(func() time.Duration); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(time.Duration) - } - - return r0 -} - -// NodePool provides a mock function with given fields: -func (_m *EVM) NodePool() config.NodePool { - ret := _m.Called() - - var r0 config.NodePool - if rf, ok := ret.Get(0).(func() config.NodePool); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.NodePool) - } - } - - return r0 -} - -// NonceAutoSync provides a mock function with given fields: -func (_m *EVM) NonceAutoSync() bool { - ret := _m.Called() - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// OCR provides a mock function with given fields: -func (_m *EVM) OCR() config.OCR { - ret := _m.Called() - - var r0 config.OCR - if rf, ok := ret.Get(0).(func() config.OCR); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.OCR) - } - } - - return r0 -} - -// OCR2 provides a mock function with given fields: -func (_m *EVM) OCR2() config.OCR2 { - ret := _m.Called() - - var r0 config.OCR2 - if rf, ok := ret.Get(0).(func() config.OCR2); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.OCR2) - } - } - - return r0 -} - -// OperatorFactoryAddress provides a mock function with given fields: -func (_m *EVM) OperatorFactoryAddress() string { - ret := _m.Called() - - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(string) - } - - return r0 -} - -// RPCDefaultBatchSize provides a mock function with given fields: -func (_m *EVM) RPCDefaultBatchSize() uint32 { - ret := _m.Called() - - var r0 uint32 - if rf, ok := ret.Get(0).(func() uint32); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(uint32) - } - - return r0 -} - -// TOMLString provides a mock function with given fields: -func (_m *EVM) TOMLString() (string, error) { - ret := _m.Called() - - var r0 string - var r1 error - if rf, ok := ret.Get(0).(func() (string, error)); ok { - return rf() - } - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(string) - } - - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// Transactions provides a mock function with given fields: -func (_m *EVM) Transactions() config.Transactions { - ret := _m.Called() - - var r0 config.Transactions - if rf, ok := ret.Get(0).(func() config.Transactions); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.Transactions) - } - } - - return r0 -} - -// NewEVM creates a new instance of EVM. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewEVM(t interface { - mock.TestingT - Cleanup(func()) -}) *EVM { - mock := &EVM{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 9f679f4a1c3..2b850a7de47 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -310,14 +310,12 @@ func TestTransactionsController_Create(t *testing.T) { respError.Error()) }) - newChain := func(t *testing.T, txm txmgr.TxManager, limitDefault uint32) evm.Chain { + _, _, evmConfig := txmgr.MakeTestConfigs(t) + feeLimit := evmConfig.GasEstimator().LimitDefault() + newChain := func(t *testing.T, txm txmgr.TxManager) evm.Chain { chain := evmMocks.NewChain(t) chain.On("TxManager").Return(txm) - // gas estimator default limit - gasEstimator := evmConfigMocks.NewGasEstimator(t) - gasEstimator.On("LimitDefault").Return(limitDefault).Maybe() - evmConfig := evmConfigMocks.NewEVM(t) - evmConfig.On("GasEstimator").Return(gasEstimator).Maybe() + config := evmConfigMocks.NewChainScopedConfig(t) config.On("EVM").Return(evmConfig).Maybe() chain.On("Config").Return(config).Maybe() @@ -327,8 +325,8 @@ func TestTransactionsController_Create(t *testing.T) { t.Run("Correctly populates fields for TxRequest", func(t *testing.T) { payload := []byte("tx_payload") value := big.NewInt(rand.Int64()) - feeLimit := rand.Uint32() + feeLimitOverride := feeLimit + 10 request := models.CreateEVMTransactionRequest{ ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), FromAddress: common.HexToAddress("0x39364605296d7c77e7C2089F0e48D527bb309d38"), @@ -337,7 +335,7 @@ func TestTransactionsController_Create(t *testing.T) { ChainID: chainID, Value: utils.NewBig(value), ForwarderAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), - FeeLimit: feeLimit, + FeeLimit: feeLimitOverride, } txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) @@ -348,13 +346,13 @@ func TestTransactionsController_Create(t *testing.T) { ToAddress: *request.ToAddress, EncodedPayload: payload, Value: *value, - FeeLimit: feeLimit, + FeeLimit: feeLimitOverride, ForwarderAddress: request.ForwarderAddress, Strategy: txmgrcommon.NewSendEveryStrategy(), }).Return(txmgr.Tx{}, expectedError).Once() chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm, 0) + chain := newChain(t, txm) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() ethKeystore := ksMocks.NewEth(t) @@ -383,19 +381,18 @@ func TestTransactionsController_Create(t *testing.T) { txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) expectedError := errors.New("stub error to shortcut execution") - expectedFeeLimit := rand.Uint32() txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ IdempotencyKey: &request.IdempotencyKey, FromAddress: expectedFromAddress, ToAddress: *request.ToAddress, EncodedPayload: []byte{}, Value: big.Int{}, - FeeLimit: expectedFeeLimit, + FeeLimit: feeLimit, Strategy: txmgrcommon.NewSendEveryStrategy(), }).Return(txmgr.Tx{}, expectedError).Once() chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm, expectedFeeLimit) + chain := newChain(t, txm) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() resp := createTx(&web.EvmTransactionController{ @@ -411,14 +408,13 @@ func TestTransactionsController_Create(t *testing.T) { }) payload := []byte("tx_payload") - expectedFeeLimit := uint32(2235235) const txID = int64(54323) newTxFromRequest := func(request models.CreateEVMTransactionRequest) txmgr.Tx { return txmgr.Tx{ ID: txID, EncodedPayload: payload, FromAddress: request.FromAddress, - FeeLimit: expectedFeeLimit, + FeeLimit: feeLimit, State: txmgrcommon.TxInProgress, ToAddress: *request.ToAddress, Value: *request.Value.ToInt(), @@ -434,7 +430,7 @@ func TestTransactionsController_Create(t *testing.T) { ToAddress: *request.ToAddress, EncodedPayload: payload, Value: *request.Value.ToInt(), - FeeLimit: expectedFeeLimit, + FeeLimit: feeLimit, Strategy: txmgrcommon.NewSendEveryStrategy(), }).Return(tx, nil).Once() return txm @@ -454,7 +450,7 @@ func TestTransactionsController_Create(t *testing.T) { txm := newTxManager(request) chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm, expectedFeeLimit) + chain := newChain(t, txm) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() txmStorage := txmEvmMocks.NewEvmTxStore(t) @@ -489,7 +485,7 @@ func TestTransactionsController_Create(t *testing.T) { txm := newTxManager(request) chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm, expectedFeeLimit) + chain := newChain(t, txm) chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() block := int64(56345431) txWithAttempts := newTxFromRequest(request) From c70bb5d22b1a9608d5fa0d59ce83fc9edd5f1f46 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 19:48:41 +0100 Subject: [PATCH 13/16] handle fee bump --- core/chains/evm/txmgr/evm_tx_store.go | 37 ++++++-- core/chains/evm/txmgr/evm_tx_store_test.go | 20 +++- core/chains/evm/txmgr/mocks/evm_tx_store.go | 24 ++--- core/internal/cltest/factories.go | 2 + core/scripts/go.mod | 3 + core/scripts/go.sum | 6 ++ core/web/evm_transactions_controller.go | 55 ++++++----- core/web/evm_transactions_controller_test.go | 96 +++++++++++--------- go.mod | 6 ++ go.sum | 6 ++ integration-tests/go.mod | 3 + integration-tests/go.sum | 6 ++ 12 files changed, 169 insertions(+), 95 deletions(-) diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index c3371fee80b..2d28beaf0a3 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -10,6 +10,7 @@ import ( "strings" "time" + sq "github.com/Masterminds/squirrel" "github.com/ethereum/go-ethereum/common" "github.com/google/uuid" "github.com/jackc/pgconn" @@ -55,7 +56,7 @@ type TxStoreWebApi interface { FindTxByHash(hash common.Hash) (*Tx, error) Transactions(offset, limit int) ([]Tx, int, error) TxAttempts(offset, limit int) ([]TxAttempt, int, error) - TransactionsWithAttempts(offset, limit int) ([]Tx, int, error) + TransactionsWithAttempts(selector TransactionsWithAttemptsSelector) ([]Tx, int, error) FindTxAttempt(hash common.Hash) (*TxAttempt, error) FindTxWithAttempts(etxID int64) (etx Tx, err error) } @@ -440,18 +441,38 @@ func (o *evmTxStore) Transactions(offset, limit int) (txs []Tx, count int, err e return } +type TransactionsWithAttemptsSelector struct { + IdempotencyKey *string + Offset uint64 + Limit uint64 +} + // TransactionsWithAttempts returns all eth transactions with at least one attempt // limited by passed parameters. Attempts are sorted by id. -func (o *evmTxStore) TransactionsWithAttempts(offset, limit int) (txs []Tx, count int, err error) { - sql := `SELECT count(*) FROM evm.txes WHERE id IN (SELECT DISTINCT eth_tx_id FROM evm.tx_attempts)` - if err = o.q.Get(&count, sql); err != nil { - return +func (o *evmTxStore) TransactionsWithAttempts(selector TransactionsWithAttemptsSelector) (txs []Tx, count int, err error) { + stmt := sq.Select("count(*)").From("evm.txes"). + Where("id IN (SELECT DISTINCT eth_tx_id FROM evm.tx_attempts)").PlaceholderFormat(sq.Dollar) + if selector.IdempotencyKey != nil { + stmt = stmt.Where("idempotency_key = ?", *selector.IdempotencyKey) } - sql = `SELECT * FROM evm.txes WHERE id IN (SELECT DISTINCT eth_tx_id FROM evm.tx_attempts) ORDER BY id desc LIMIT $1 OFFSET $2` + query, args, err := stmt.ToSql() + if err != nil { + return nil, 0, fmt.Errorf("failed to build count query: %w", err) + } + + if err = o.q.Get(&count, query, args...); err != nil { + return nil, 0, fmt.Errorf("failed to exec count query: %w, sql: %s", err, query) + } + + query, args, err = stmt.RemoveColumns().Columns("*"). + OrderBy("id desc").Limit(selector.Limit).Offset(selector.Offset).ToSql() + if err != nil { + return nil, 0, fmt.Errorf("failed to build select query: %w", err) + } var dbTxs []DbEthTx - if err = o.q.Select(&dbTxs, sql, limit, offset); err != nil { - return + if err = o.q.Select(&dbTxs, query, args...); err != nil { + return nil, 0, fmt.Errorf("failed to exec select query: %w, sql: %s", err, query) } txs = dbEthTxsToEvmEthTxs(dbTxs) err = o.preloadTxAttempts(txs) diff --git a/core/chains/evm/txmgr/evm_tx_store_test.go b/core/chains/evm/txmgr/evm_tx_store_test.go index f8798f9f836..8a7a50b4d37 100644 --- a/core/chains/evm/txmgr/evm_tx_store_test.go +++ b/core/chains/evm/txmgr/evm_tx_store_test.go @@ -59,7 +59,10 @@ func TestORM_TransactionsWithAttempts(t *testing.T) { require.NoError(t, err) require.Equal(t, 3, count) - txs, count, err := txStore.TransactionsWithAttempts(0, 100) // should omit tx3 + txs, count, err := txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + Offset: 0, + Limit: 100, + }) // should omit tx3 require.NoError(t, err) assert.Equal(t, 2, count, "only eth txs with attempts are counted") assert.Len(t, txs, 2) @@ -70,11 +73,24 @@ func TestORM_TransactionsWithAttempts(t *testing.T) { assert.Equal(t, int64(3), *txs[0].TxAttempts[0].BroadcastBeforeBlockNum, "attempts should be sorted by created_at") assert.Equal(t, int64(2), *txs[0].TxAttempts[1].BroadcastBeforeBlockNum, "attempts should be sorted by created_at") - txs, count, err = txStore.TransactionsWithAttempts(0, 1) + txs, count, err = txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + Offset: 0, + Limit: 1, + }) require.NoError(t, err) assert.Equal(t, 2, count, "only eth txs with attempts are counted") assert.Len(t, txs, 1, "limit should apply to length of results") assert.Equal(t, evmtypes.Nonce(1), *txs[0].Sequence, "transactions should be sorted by nonce") + + txs, count, err = txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + IdempotencyKey: tx2.IdempotencyKey, + Offset: 0, + Limit: 10, + }) + require.NoError(t, err) + assert.Equal(t, 1, count, "only eth tx with specified idempotency key") + assert.Equal(t, tx2.ID, txs[0].ID) + assert.Equal(t, tx2.IdempotencyKey, txs[0].IdempotencyKey) } func TestORM_Transactions(t *testing.T) { diff --git a/core/chains/evm/txmgr/mocks/evm_tx_store.go b/core/chains/evm/txmgr/mocks/evm_tx_store.go index f491bda40bb..d528de819ad 100644 --- a/core/chains/evm/txmgr/mocks/evm_tx_store.go +++ b/core/chains/evm/txmgr/mocks/evm_tx_store.go @@ -16,6 +16,8 @@ import ( time "time" + txmgr "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + types "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" uuid "github.com/google/uuid" @@ -924,32 +926,32 @@ func (_m *EvmTxStore) Transactions(offset int, limit int) ([]types.Tx[*big.Int, return r0, r1, r2 } -// TransactionsWithAttempts provides a mock function with given fields: offset, limit -func (_m *EvmTxStore) TransactionsWithAttempts(offset int, limit int) ([]types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], int, error) { - ret := _m.Called(offset, limit) +// TransactionsWithAttempts provides a mock function with given fields: selector +func (_m *EvmTxStore) TransactionsWithAttempts(selector txmgr.TransactionsWithAttemptsSelector) ([]types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], int, error) { + ret := _m.Called(selector) var r0 []types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee] var r1 int var r2 error - if rf, ok := ret.Get(0).(func(int, int) ([]types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], int, error)); ok { - return rf(offset, limit) + if rf, ok := ret.Get(0).(func(txmgr.TransactionsWithAttemptsSelector) ([]types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee], int, error)); ok { + return rf(selector) } - if rf, ok := ret.Get(0).(func(int, int) []types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]); ok { - r0 = rf(offset, limit) + if rf, ok := ret.Get(0).(func(txmgr.TransactionsWithAttemptsSelector) []types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]); ok { + r0 = rf(selector) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]types.Tx[*big.Int, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee]) } } - if rf, ok := ret.Get(1).(func(int, int) int); ok { - r1 = rf(offset, limit) + if rf, ok := ret.Get(1).(func(txmgr.TransactionsWithAttemptsSelector) int); ok { + r1 = rf(selector) } else { r1 = ret.Get(1).(int) } - if rf, ok := ret.Get(2).(func(int, int) error); ok { - r2 = rf(offset, limit) + if rf, ok := ret.Get(2).(func(txmgr.TransactionsWithAttemptsSelector) error); ok { + r2 = rf(selector) } else { r2 = ret.Error(2) } diff --git a/core/internal/cltest/factories.go b/core/internal/cltest/factories.go index a52b9a5d06b..202f9aeecf3 100644 --- a/core/internal/cltest/factories.go +++ b/core/internal/cltest/factories.go @@ -135,7 +135,9 @@ func EmptyCLIContext() *cli.Context { } func NewEthTx(t *testing.T, fromAddress common.Address) txmgr.Tx { + idempotencyKey := uuid.New().String() return txmgr.Tx{ + IdempotencyKey: &idempotencyKey, FromAddress: fromAddress, ToAddress: testutils.NewAddress(), EncodedPayload: []byte{1, 2, 3}, diff --git a/core/scripts/go.mod b/core/scripts/go.mod index eb41312a6ab..911407d3aa8 100644 --- a/core/scripts/go.mod +++ b/core/scripts/go.mod @@ -51,6 +51,7 @@ require ( github.com/DataDog/zstd v1.5.2 // indirect github.com/Depado/ginprom v1.7.11 // indirect github.com/Masterminds/semver/v3 v3.2.1 // indirect + github.com/Masterminds/squirrel v1.5.4 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect github.com/VictoriaMetrics/fastcache v1.10.0 // indirect github.com/andres-erbsen/clock v0.0.0-20160526145045-9e14626cd129 // indirect @@ -207,6 +208,8 @@ require ( github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect + github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect + github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/lib/pq v1.10.9 // indirect diff --git a/core/scripts/go.sum b/core/scripts/go.sum index 35e85fe2c97..d467eda000e 100644 --- a/core/scripts/go.sum +++ b/core/scripts/go.sum @@ -105,6 +105,8 @@ github.com/Kubuxu/go-os-helper v0.0.1/go.mod h1:N8B+I7vPCT80IcP58r50u4+gEEcsZETF github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= +github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= +github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= @@ -919,6 +921,10 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index bde170f7781..ede7acd07be 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -1,11 +1,9 @@ package web import ( - "context" "database/sql" "math/big" "net/http" - "time" "github.com/ethereum/go-ethereum/common/hexutil" @@ -32,7 +30,16 @@ type TransactionsController struct { // Index returns paginated transactions func (tc *TransactionsController) Index(c *gin.Context, size, page, offset int) { - txs, count, err := tc.App.TxmStorageService().TransactionsWithAttempts(offset, size) + var idempotencyKey *string + rawIdempotencyKey := c.Query("idempotencyKey") + if rawIdempotencyKey != "" { + idempotencyKey = &rawIdempotencyKey + } + txs, count, err := tc.App.TxmStorageService().TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + IdempotencyKey: idempotencyKey, + Offset: uint64(offset), + Limit: uint64(size), + }) ptxs := make([]presenters.EthTxResource, len(txs)) for i, tx := range txs { tx.TxAttempts[0].Tx = tx @@ -62,24 +69,20 @@ func (tc *TransactionsController) Show(c *gin.Context) { } type EvmTransactionController struct { - Enabled bool - Logger logger.SugaredLogger - TxmStore txmgr.EvmTxStore - AuditLogger audit.AuditLogger - Chains evm.LegacyChainContainer - KeyStore keystore.Eth - TxAttemptWaitDur time.Duration + Enabled bool + Logger logger.SugaredLogger + AuditLogger audit.AuditLogger + Chains evm.LegacyChainContainer + KeyStore keystore.Eth } func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ - Enabled: app.GetConfig().TxmAsService().Enabled(), - TxmStore: app.TxmStorageService(), - Logger: app.GetLogger(), - AuditLogger: app.GetAuditLogger(), - Chains: app.GetRelayers().LegacyEVMChains(), - KeyStore: app.GetKeyStore().Eth(), - TxAttemptWaitDur: 10 * time.Second, + Enabled: app.GetConfig().TxmAsService().Enabled(), + Logger: app.GetLogger(), + AuditLogger: app.GetAuditLogger(), + Chains: app.GetRelayers().LegacyEVMChains(), + KeyStore: app.GetKeyStore().Eth(), } } @@ -171,16 +174,10 @@ func (tc *EvmTransactionController) Create(c *gin.Context) { "ethTX": etx, }) - // wait and retrieve tx attempt matching tx ID - attempt, err := FindTxAttempt(c, tc.TxAttemptWaitDur, etx, tc.TxmStore.FindTxWithAttempts) - if err != nil { - if errors.Is(err, context.DeadlineExceeded) { - jsonAPIError(c, http.StatusGatewayTimeout, errors.Errorf("failed to find transaction within timeout: %v", err)) - return - } - - jsonAPIError(c, http.StatusInternalServerError, errors.Errorf("failed to find transaction: %v", err)) - return - } - jsonAPIResponse(c, presenters.NewEthTxResourceFromAttempt(attempt), "eth_tx") + // We have successfully accepted user's request, but have noting to return at the moment. + // We deliberately avoid using EthTxResource here due to its misleading design. It has ID of `txmgr.TxAttempt` and + // state of `txmgr.Tx`, so caller might end up with resource that has ID equal to hash of tx attempt that was + // not included into the chain, while `attributes.state` is `confirmed`. + // User is expected to track transaction status using IdempotencyKey and `/v2/transactions/evm`. + c.Status(http.StatusAccepted) } diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 2b850a7de47..2f916066449 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -24,7 +24,6 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" - txmEvmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" @@ -64,7 +63,10 @@ func TestTransactionsController_Index_Success(t *testing.T) { attempt.BroadcastBeforeBlockNum = &blockNum require.NoError(t, txStore.InsertTxAttempt(&attempt)) - _, count, err := txStore.TransactionsWithAttempts(0, 100) + _, count, err := txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + Offset: 0, + Limit: 100, + }) require.NoError(t, err) require.Equal(t, count, 3) @@ -85,6 +87,49 @@ func TestTransactionsController_Index_Success(t *testing.T) { require.Equal(t, "3", txs[1].SentAt, "expected tx attempts order by sentAt descending") } +func TestTransactionsController_Index_Success_IdempotencyKey(t *testing.T) { + t.Parallel() + + app := cltest.NewApplicationWithKey(t) + require.NoError(t, app.Start(testutils.Context(t))) + + db := app.GetSqlxDB() + txStore := cltest.NewTestTxStore(t, app.GetSqlxDB(), app.GetConfig().Database()) + ethKeyStore := cltest.NewKeyStore(t, db, app.Config.Database()).Eth() + client := app.NewHTTPClient(nil) + _, from := cltest.MustInsertRandomKey(t, ethKeyStore) + + cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, txStore, 0, 1, from) + tx := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, txStore, 3, 2, from) + + // add second tx attempt for tx + blockNum := int64(3) + attempt := cltest.NewLegacyEthTxAttempt(t, tx.ID) + attempt.State = txmgrtypes.TxAttemptBroadcast + attempt.TxFee = gas.EvmFee{Legacy: assets.NewWeiI(3)} + attempt.BroadcastBeforeBlockNum = &blockNum + require.NoError(t, txStore.InsertTxAttempt(&attempt)) + + _, count, err := txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + Offset: 0, + Limit: 100, + }) + require.NoError(t, err) + require.Equal(t, count, 2) + + resp, cleanup := client.Get(fmt.Sprintf("/v2/transactions?size=%d&idempotencyKey=%s", 10, *tx.IdempotencyKey)) + t.Cleanup(cleanup) + cltest.AssertServerResponse(t, resp, http.StatusOK) + + var links jsonapi.Links + var txs []presenters.EthTxResource + body := cltest.ParseResponseBody(t, resp) + require.NoError(t, web.ParsePaginatedResponse(body, &txs, &links)) + + require.Len(t, txs, 1) + require.Equal(t, attempt.Hash, txs[0].Hash, "expected tx attempts order by sentAt descending") +} + func TestTransactionsController_Index_Error(t *testing.T) { t.Parallel() @@ -435,40 +480,6 @@ func TestTransactionsController_Create(t *testing.T) { }).Return(tx, nil).Once() return txm } - t.Run("Fails to find tx attempt in time", func(t *testing.T) { - request := models.CreateEVMTransactionRequest{ - FromAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), - ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), - IdempotencyKey: "idempotency_key", - EncodedPayload: "0x" + fmt.Sprintf("%X", payload), - ChainID: chainID, - Value: utils.NewBigI(6838712), - } - - ethKeystore := ksMocks.NewEth(t) - ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() - txm := newTxManager(request) - - chainContainer := evmMocks.NewLegacyChainContainer(t) - chain := newChain(t, txm) - chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() - - txmStorage := txmEvmMocks.NewEvmTxStore(t) - txmStorage.On("FindTxWithAttempts", txID).Return(txmgr.Tx{}, nil).Maybe() - - resp := createTx(&web.EvmTransactionController{ - AuditLogger: audit.NoopLogger, - Chains: chainContainer, - KeyStore: ethKeystore, - TxmStore: txmStorage, - TxAttemptWaitDur: testutils.TestInterval, - }, request).Result() - - cltest.AssertServerResponse(t, resp, http.StatusGatewayTimeout) - respError := cltest.ParseJSONAPIErrors(t, resp.Body) - require.Equal(t, "failed to find transaction within timeout: context deadline exceeded - tx may still have been broadcast", - respError.Error()) - }) t.Run("Happy path", func(t *testing.T) { request := models.CreateEVMTransactionRequest{ FromAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), @@ -496,17 +507,12 @@ func TestTransactionsController_Create(t *testing.T) { }, } - txmStorage := txmEvmMocks.NewEvmTxStore(t) - txmStorage.On("FindTxWithAttempts", txWithAttempts.ID).Return(txWithAttempts, nil) - resp := createTx(&web.EvmTransactionController{ - AuditLogger: audit.NoopLogger, - Chains: chainContainer, - KeyStore: ethKeystore, - TxmStore: txmStorage, - TxAttemptWaitDur: testutils.DefaultWaitTimeout, + AuditLogger: audit.NoopLogger, + Chains: chainContainer, + KeyStore: ethKeystore, }, request).Result() - cltest.AssertServerResponse(t, resp, http.StatusOK) + cltest.AssertServerResponse(t, resp, http.StatusAccepted) }) } diff --git a/go.mod b/go.mod index 0a85fe7f488..dbe84c3ac4f 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/Depado/ginprom v1.7.11 github.com/Masterminds/semver/v3 v3.2.1 github.com/Masterminds/sprig/v3 v3.2.3 + github.com/Masterminds/squirrel v1.5.4 github.com/avast/retry-go/v4 v4.5.0 github.com/btcsuite/btcd v0.23.4 github.com/cometbft/cometbft v0.37.2 @@ -103,6 +104,11 @@ require ( gopkg.in/natefinch/lumberjack.v2 v2.0.0 ) +require ( + github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect + github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect +) + require ( contrib.go.opencensus.io/exporter/stackdriver v0.13.5 // indirect cosmossdk.io/api v0.3.1 // indirect diff --git a/go.sum b/go.sum index 7fe91a6b123..79ecf044447 100644 --- a/go.sum +++ b/go.sum @@ -110,6 +110,8 @@ github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0 github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= github.com/Masterminds/sprig/v3 v3.2.3 h1:eL2fZNezLomi0uOLqjQoN6BfsDD+fyLtgbJMAj9n6YA= github.com/Masterminds/sprig/v3 v3.2.3/go.mod h1:rXcFaZ2zZbLRJv/xSysmlgIM1u11eBaRMhvYXJNkGuM= +github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= +github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= @@ -920,6 +922,10 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= diff --git a/integration-tests/go.mod b/integration-tests/go.mod index a943e1c41a9..d9a8eec6dfe 100644 --- a/integration-tests/go.mod +++ b/integration-tests/go.mod @@ -65,6 +65,7 @@ require ( github.com/K-Phoen/sdk v0.12.2 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect github.com/Masterminds/semver/v3 v3.2.1 // indirect + github.com/Masterminds/squirrel v1.5.4 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect github.com/VictoriaMetrics/fastcache v1.10.0 // indirect github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect @@ -268,6 +269,8 @@ require ( github.com/koron/go-ssdp v0.0.2 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect + github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect + github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/libp2p/go-addr-util v0.0.2 // indirect diff --git a/integration-tests/go.sum b/integration-tests/go.sum index 5719c36b5a8..85dfacd904a 100644 --- a/integration-tests/go.sum +++ b/integration-tests/go.sum @@ -610,6 +610,8 @@ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6 github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= +github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= +github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Microsoft/hcsshim v0.10.0-rc.8 h1:YSZVvlIIDD1UxQpJp0h+dnpLUw+TrY0cx8obKsp3bek= @@ -1709,6 +1711,10 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= +github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= +github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= From 0c4af4ec40a51607c54d1e229f7722048896f8a2 Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Thu, 16 Nov 2023 20:20:52 +0100 Subject: [PATCH 14/16] fixed merge --- core/web/evm_transactions_controller_test.go | 387 +++++++++++++++++++ 1 file changed, 387 insertions(+) diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 72e85d34c47..5304209b181 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -1,16 +1,36 @@ package web_test import ( + "bytes" + "encoding/json" "fmt" + "math/big" "net/http" + "net/http/httptest" "testing" + "github.com/cometbft/cometbft/libs/rand" + "github.com/ethereum/go-ethereum/common" + "github.com/gin-gonic/gin" + "github.com/pkg/errors" + "github.com/stretchr/testify/mock" + + txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" + txmMocks "github.com/smartcontractkit/chainlink/v2/common/txmgr/mocks" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + evmConfigMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + evmMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/internal/cltest" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger/audit" + ksMocks "github.com/smartcontractkit/chainlink/v2/core/services/keystore/mocks" + "github.com/smartcontractkit/chainlink/v2/core/store/models" + "github.com/smartcontractkit/chainlink/v2/core/utils" "github.com/smartcontractkit/chainlink/v2/core/web" "github.com/smartcontractkit/chainlink/v2/core/web/presenters" @@ -67,6 +87,49 @@ func TestTransactionsController_Index_Success(t *testing.T) { require.Equal(t, "3", txs[1].SentAt, "expected tx attempts order by sentAt descending") } +func TestTransactionsController_Index_Success_IdempotencyKey(t *testing.T) { + t.Parallel() + + app := cltest.NewApplicationWithKey(t) + require.NoError(t, app.Start(testutils.Context(t))) + + db := app.GetSqlxDB() + txStore := cltest.NewTestTxStore(t, app.GetSqlxDB(), app.GetConfig().Database()) + ethKeyStore := cltest.NewKeyStore(t, db, app.Config.Database()).Eth() + client := app.NewHTTPClient(nil) + _, from := cltest.MustInsertRandomKey(t, ethKeyStore) + + cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, txStore, 0, 1, from) + tx := cltest.MustInsertConfirmedEthTxWithLegacyAttempt(t, txStore, 3, 2, from) + + // add second tx attempt for tx + blockNum := int64(3) + attempt := cltest.NewLegacyEthTxAttempt(t, tx.ID) + attempt.State = txmgrtypes.TxAttemptBroadcast + attempt.TxFee = gas.EvmFee{Legacy: assets.NewWeiI(3)} + attempt.BroadcastBeforeBlockNum = &blockNum + require.NoError(t, txStore.InsertTxAttempt(&attempt)) + + _, count, err := txStore.TransactionsWithAttempts(txmgr.TransactionsWithAttemptsSelector{ + Offset: 0, + Limit: 100, + }) + require.NoError(t, err) + require.Equal(t, count, 2) + + resp, cleanup := client.Get(fmt.Sprintf("/v2/transactions?size=%d&idempotencyKey=%s", 10, *tx.IdempotencyKey)) + t.Cleanup(cleanup) + cltest.AssertServerResponse(t, resp, http.StatusOK) + + var links jsonapi.Links + var txs []presenters.EthTxResource + body := cltest.ParseResponseBody(t, resp) + require.NoError(t, web.ParsePaginatedResponse(body, &txs, &links)) + + require.Len(t, txs, 1) + require.Equal(t, attempt.Hash, txs[0].Hash, "expected tx attempts order by sentAt descending") +} + func TestTransactionsController_Index_Error(t *testing.T) { t.Parallel() @@ -129,3 +192,327 @@ func TestTransactionsController_Show_NotFound(t *testing.T) { t.Cleanup(cleanup) cltest.AssertServerResponse(t, resp, http.StatusNotFound) } + +func TestTransactionsController_Create(t *testing.T) { + t.Parallel() + const txCreatePath = "/v2/transactions/evm" + t.Run("Returns error if endpoint is disabled", func(t *testing.T) { + req, _ := http.NewRequest(http.MethodPost, txCreatePath, nil) + router := gin.New() + controller := &web.EvmTransactionController{ + Enabled: false, + } + router.POST(txCreatePath, controller.Create) + resp := httptest.NewRecorder() + router.ServeHTTP(resp, req) + cltest.AssertServerResponse(t, resp.Result(), http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "transactions creation disabled. To enable set TxmAsService.Enabled=true", respError.Error()) + }) + + createTx := func(controller *web.EvmTransactionController, request interface{}) *httptest.ResponseRecorder { + body, err := json.Marshal(&request) + assert.NoError(t, err) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPost, txCreatePath, bytes.NewBuffer(body)) + router := gin.New() + controller.Enabled = true + router.POST(txCreatePath, controller.Create) + router.ServeHTTP(w, req) + return w + } + + t.Run("Fails on malformed json", func(t *testing.T) { + resp := createTx(&web.EvmTransactionController{}, "Hello") + + cltest.AssertServerResponse(t, resp.Result(), http.StatusBadRequest) + }) + t.Run("Fails on missing Idempotency key", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + } + + resp := createTx(&web.EvmTransactionController{}, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "idempotencyKey must be set", respError.Error()) + }) + t.Run("Fails on malformed payload", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + } + + resp := createTx(&web.EvmTransactionController{}, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "encodedPayload is malformed: empty hex string", respError.Error()) + }) + t.Run("Fails if chain ID is not set", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + } + + resp := createTx(&web.EvmTransactionController{}, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "chainID must be set", respError.Error()) + }) + t.Run("Fails if toAddress is not specified", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: nil, + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: utils.NewBigI(0), + } + + resp := createTx(&web.EvmTransactionController{}, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusBadRequest) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "toAddress must be set", respError.Error()) + }) + chainID := utils.NewBigI(673728) + t.Run("Fails if requested chain that is not available", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: common.HexToAddress("0x0000000000000000000000000000000000000000"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chainContainer.On("Get", chainID.String()).Return(nil, web.ErrMissingChainID).Once() + controller := &web.EvmTransactionController{ + Chains: chainContainer, + } + resp := createTx(controller, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, web.ErrMissingChainID.Error(), respError.Error()) + }) + t.Run("Fails when fromAddress is not specified and there are no available keys ", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := evmMocks.NewChain(t) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()). + Return(nil, errors.New("failed to get key")).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, "failed to get fromAddress: failed to get key", respError.Error()) + }) + t.Run("Fails when specified fromAddress is not available for the chain", func(t *testing.T) { + fromAddr := common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371") + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xFA01FA015C8A5332987319823728982379128371")), + FromAddress: fromAddr, + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := evmMocks.NewChain(t) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("CheckEnabled", fromAddr, chainID.ToInt()). + Return(errors.New("no eth key exists with address")).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusUnprocessableEntity) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, + "fromAddress 0xfa01fA015c8A5332987319823728982379128371 is not available: no eth key exists with address", + respError.Error()) + }) + + _, _, evmConfig := txmgr.MakeTestConfigs(t) + feeLimit := evmConfig.GasEstimator().LimitDefault() + newChain := func(t *testing.T, txm txmgr.TxManager) evm.Chain { + chain := evmMocks.NewChain(t) + chain.On("TxManager").Return(txm) + + config := evmConfigMocks.NewChainScopedConfig(t) + config.On("EVM").Return(evmConfig).Maybe() + chain.On("Config").Return(config).Maybe() + + return chain + } + t.Run("Correctly populates fields for TxRequest", func(t *testing.T) { + payload := []byte("tx_payload") + value := big.NewInt(rand.Int64()) + + feeLimitOverride := feeLimit + 10 + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + FromAddress: common.HexToAddress("0x39364605296d7c77e7C2089F0e48D527bb309d38"), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), + ChainID: chainID, + Value: utils.NewBig(value), + ForwarderAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + FeeLimit: feeLimitOverride, + } + + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + expectedError := errors.New("stub error to shortcut execution") + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: request.FromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: payload, + Value: *value, + FeeLimit: feeLimitOverride, + ForwarderAddress: request.ForwarderAddress, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(txmgr.Tx{}, expectedError).Once() + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusInternalServerError) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), + respError.Error()) + }) + t.Run("Correctly populates fields for TxRequest with defaults", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x", + ChainID: chainID, + } + + expectedFromAddress := common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2") + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("GetRoundRobinAddress", chainID.ToInt()).Return(expectedFromAddress, nil).Once() + + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + expectedError := errors.New("stub error to shortcut execution") + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: expectedFromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: []byte{}, + Value: big.Int{}, + FeeLimit: feeLimit, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(txmgr.Tx{}, expectedError).Once() + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + + resp := createTx(&web.EvmTransactionController{ + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + // we do not really care about results here as main check is during CreateTransaction call + cltest.AssertServerResponse(t, resp, http.StatusInternalServerError) + respError := cltest.ParseJSONAPIErrors(t, resp.Body) + require.Equal(t, fmt.Sprintf("transaction failed: %v", expectedError), + respError.Error()) + }) + + payload := []byte("tx_payload") + const txID = int64(54323) + newTxFromRequest := func(request models.CreateEVMTransactionRequest) txmgr.Tx { + return txmgr.Tx{ + ID: txID, + EncodedPayload: payload, + FromAddress: request.FromAddress, + FeeLimit: feeLimit, + State: txmgrcommon.TxInProgress, + ToAddress: *request.ToAddress, + Value: *request.Value.ToInt(), + ChainID: chainID.ToInt(), + } + } + newTxManager := func(request models.CreateEVMTransactionRequest) txmgr.TxManager { + txm := txmMocks.NewTxManager[*big.Int, *evmtypes.Head, common.Address, common.Hash, common.Hash, evmtypes.Nonce, gas.EvmFee](t) + tx := newTxFromRequest(request) + txm.On("CreateTransaction", mock.Anything, txmgr.TxRequest{ + IdempotencyKey: &request.IdempotencyKey, + FromAddress: request.FromAddress, + ToAddress: *request.ToAddress, + EncodedPayload: payload, + Value: *request.Value.ToInt(), + FeeLimit: feeLimit, + Strategy: txmgrcommon.NewSendEveryStrategy(), + }).Return(tx, nil).Once() + return txm + } + t.Run("Happy path", func(t *testing.T) { + request := models.CreateEVMTransactionRequest{ + FromAddress: common.HexToAddress("0x59C2B3875797c521396e7575D706B9188894eAF2"), + ToAddress: ptr(common.HexToAddress("0xEA746B853DcFFA7535C64882E191eE31BE8CE711")), + IdempotencyKey: "idempotency_key", + EncodedPayload: "0x" + fmt.Sprintf("%X", payload), + ChainID: chainID, + Value: utils.NewBigI(6838712), + } + + ethKeystore := ksMocks.NewEth(t) + ethKeystore.On("CheckEnabled", request.FromAddress, chainID.ToInt()).Return(nil).Once() + + txm := newTxManager(request) + + chainContainer := evmMocks.NewLegacyChainContainer(t) + chain := newChain(t, txm) + chainContainer.On("Get", chainID.String()).Return(chain, nil).Once() + block := int64(56345431) + txWithAttempts := newTxFromRequest(request) + txWithAttempts.TxAttempts = []txmgr.TxAttempt{ + { + Hash: common.HexToHash("0xa1ce83ee556cbcfc6541d5909b0d7f28f6a77399d3bd4340246f684a0f25a7f5"), + BroadcastBeforeBlockNum: &block, + }, + } + + resp := createTx(&web.EvmTransactionController{ + AuditLogger: audit.NoopLogger, + Chains: chainContainer, + KeyStore: ethKeystore, + }, request).Result() + + cltest.AssertServerResponse(t, resp, http.StatusAccepted) + }) +} From d8bf68550344d65326cbc6682c9acd90de56e54e Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Tue, 21 Nov 2023 16:46:29 +0100 Subject: [PATCH 15/16] switch to feature flag --- .../evm/config/mocks/chain_scoped_config.go | 16 ------------- core/config/app_config.go | 1 - core/config/docs/core.toml | 7 ++---- core/config/feature_config.go | 1 + core/config/toml/types.go | 23 +++++++------------ core/config/txm_as_service.go | 5 ---- core/services/chainlink/config_feature.go | 4 ++++ .../services/chainlink/config_feature_test.go | 1 + core/services/chainlink/config_general.go | 8 +++---- core/services/chainlink/config_test.go | 9 ++++---- .../chainlink/config_txm_as_service.go | 16 ------------- .../chainlink/mocks/general_config.go | 16 ------------- .../testdata/config-empty-effective.toml | 4 +--- .../chainlink/testdata/config-full.toml | 4 +--- .../config-multi-chain-effective.toml | 4 +--- core/web/evm_transactions_controller.go | 5 ++-- core/web/evm_transactions_controller_test.go | 2 +- .../testdata/config-empty-effective.toml | 4 +--- core/web/resolver/testdata/config-full.toml | 4 +--- .../config-multi-chain-effective.toml | 4 +--- docs/CONFIG.md | 21 ++++++----------- integration-tests/types/config/node/core.go | 1 + testdata/scripts/node/validate/default.txtar | 4 +--- .../disk-based-logging-disabled.txtar | 4 +--- .../validate/disk-based-logging-no-dir.txtar | 4 +--- .../node/validate/disk-based-logging.txtar | 4 +--- testdata/scripts/node/validate/invalid.txtar | 4 +--- testdata/scripts/node/validate/valid.txtar | 4 +--- testdata/scripts/node/validate/warnings.txtar | 4 +--- 29 files changed, 50 insertions(+), 138 deletions(-) delete mode 100644 core/config/txm_as_service.go delete mode 100644 core/services/chainlink/config_txm_as_service.go diff --git a/core/chains/evm/config/mocks/chain_scoped_config.go b/core/chains/evm/config/mocks/chain_scoped_config.go index 64c3530d66a..cb18282f495 100644 --- a/core/chains/evm/config/mocks/chain_scoped_config.go +++ b/core/chains/evm/config/mocks/chain_scoped_config.go @@ -513,22 +513,6 @@ func (_m *ChainScopedConfig) Tracing() coreconfig.Tracing { return r0 } -// TxmAsService provides a mock function with given fields: -func (_m *ChainScopedConfig) TxmAsService() coreconfig.TxmAsService { - ret := _m.Called() - - var r0 coreconfig.TxmAsService - if rf, ok := ret.Get(0).(func() coreconfig.TxmAsService); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(coreconfig.TxmAsService) - } - } - - return r0 -} - // Validate provides a mock function with given fields: func (_m *ChainScopedConfig) Validate() error { ret := _m.Called() diff --git a/core/config/app_config.go b/core/config/app_config.go index 862376b47e8..648939b871b 100644 --- a/core/config/app_config.go +++ b/core/config/app_config.go @@ -54,7 +54,6 @@ type AppConfig interface { Threshold() Threshold WebServer() WebServer Tracing() Tracing - TxmAsService() TxmAsService } type DatabaseBackupMode string diff --git a/core/config/docs/core.toml b/core/config/docs/core.toml index c2fc2560fb1..8d3842c1766 100644 --- a/core/config/docs/core.toml +++ b/core/config/docs/core.toml @@ -13,6 +13,8 @@ FeedsManager = true # Default LogPoller = false # Default # UICSAKeys enables CSA Keys in the UI. UICSAKeys = false # Default +# TransactionService enables `POST /v2/transactions/evm` endpoint. +TransactionService = false # Default [Database] # DefaultIdleInTxSessionTimeout is the maximum time allowed for a transaction to be open and idle before timing out. See Postgres `idle_in_transaction_session_timeout` for more details. @@ -597,8 +599,3 @@ SamplingRatio = 1.0 # Example [Tracing.Attributes] # env is an example user specified key-value pair env = "test" # Example - -[TxmAsService] -# Enabled turns Transaction Manager as a service feature on or off. When enabled exposes endpoint to submit arbitrary EVM transaction -# using one of the keys enabled for the specified chain. -Enabled = false # Default diff --git a/core/config/feature_config.go b/core/config/feature_config.go index fbb3a4ea541..09e7d7e608f 100644 --- a/core/config/feature_config.go +++ b/core/config/feature_config.go @@ -4,4 +4,5 @@ type Feature interface { FeedsManager() bool UICSAKeys() bool LogPoller() bool + TransactionService() bool } diff --git a/core/config/toml/types.go b/core/config/toml/types.go index d6c1990a5cd..5dbd12f1cd2 100644 --- a/core/config/toml/types.go +++ b/core/config/toml/types.go @@ -54,7 +54,6 @@ type Core struct { Sentry Sentry `toml:",omitempty"` Insecure Insecure `toml:",omitempty"` Tracing Tracing `toml:",omitempty"` - TxmAsService TxmAsService `toml:",omitempty"` } // SetFrom updates c with any non-nil values from f. (currently TOML field only!) @@ -89,7 +88,6 @@ func (c *Core) SetFrom(f *Core) { c.Sentry.setFrom(&f.Sentry) c.Insecure.setFrom(&f.Insecure) c.Tracing.setFrom(&f.Tracing) - c.TxmAsService.setFrom(&f.TxmAsService) } func (c *Core) ValidateConfig() (err error) { @@ -293,9 +291,10 @@ func (p *PrometheusSecrets) validateMerge(f *PrometheusSecrets) (err error) { } type Feature struct { - FeedsManager *bool - LogPoller *bool - UICSAKeys *bool + FeedsManager *bool + LogPoller *bool + UICSAKeys *bool + TransactionService *bool } func (f *Feature) setFrom(f2 *Feature) { @@ -308,6 +307,10 @@ func (f *Feature) setFrom(f2 *Feature) { if v := f2.UICSAKeys; v != nil { f.UICSAKeys = v } + + if v := f2.TransactionService; v != nil { + f.TransactionService = v + } } type Database struct { @@ -1532,13 +1535,3 @@ func isValidURI(uri string) bool { func isValidHostname(hostname string) bool { return hostnameRegex.MatchString(hostname) } - -type TxmAsService struct { - Enabled *bool -} - -func (t *TxmAsService) setFrom(f *TxmAsService) { - if v := f.Enabled; v != nil { - t.Enabled = f.Enabled - } -} diff --git a/core/config/txm_as_service.go b/core/config/txm_as_service.go deleted file mode 100644 index 1672a66e482..00000000000 --- a/core/config/txm_as_service.go +++ /dev/null @@ -1,5 +0,0 @@ -package config - -type TxmAsService interface { - Enabled() bool -} diff --git a/core/services/chainlink/config_feature.go b/core/services/chainlink/config_feature.go index 2e968df052d..bfd0b3eeaeb 100644 --- a/core/services/chainlink/config_feature.go +++ b/core/services/chainlink/config_feature.go @@ -17,3 +17,7 @@ func (f *featureConfig) LogPoller() bool { func (f *featureConfig) UICSAKeys() bool { return *f.c.UICSAKeys } + +func (f *featureConfig) TransactionService() bool { + return *f.c.TransactionService +} diff --git a/core/services/chainlink/config_feature_test.go b/core/services/chainlink/config_feature_test.go index bc0418c157b..0d20568f2f9 100644 --- a/core/services/chainlink/config_feature_test.go +++ b/core/services/chainlink/config_feature_test.go @@ -18,4 +18,5 @@ func TestFeatureConfig(t *testing.T) { assert.True(t, f.LogPoller()) assert.True(t, f.FeedsManager()) assert.True(t, f.UICSAKeys()) + assert.True(t, f.TransactionService()) } diff --git a/core/services/chainlink/config_general.go b/core/services/chainlink/config_general.go index d8aabefbd03..5088c75d7d2 100644 --- a/core/services/chainlink/config_general.go +++ b/core/services/chainlink/config_general.go @@ -296,6 +296,10 @@ func (g *generalConfig) FeatureUICSAKeys() bool { return *g.c.Feature.UICSAKeys } +func (g *generalConfig) FeatureTransactionService() bool { + return *g.c.Feature.TransactionService +} + func (g *generalConfig) AutoPprof() config.AutoPprof { return &autoPprofConfig{c: g.c.AutoPprof, rootDir: g.RootDir} } @@ -518,8 +522,4 @@ func (g *generalConfig) Tracing() coreconfig.Tracing { return &tracingConfig{s: g.c.Tracing} } -func (g *generalConfig) TxmAsService() config.TxmAsService { - return &txmAsServiceConfig{c: g.c.TxmAsService} -} - var zeroSha256Hash = models.Sha256Hash{} diff --git a/core/services/chainlink/config_test.go b/core/services/chainlink/config_test.go index 7f27f8460a1..322e873cfa3 100644 --- a/core/services/chainlink/config_test.go +++ b/core/services/chainlink/config_test.go @@ -250,9 +250,10 @@ func TestConfig_Marshal(t *testing.T) { } full.Feature = toml.Feature{ - FeedsManager: ptr(true), - LogPoller: ptr(true), - UICSAKeys: ptr(true), + FeedsManager: ptr(true), + LogPoller: ptr(true), + UICSAKeys: ptr(true), + TransactionService: ptr(true), } full.Database = toml.Database{ DefaultIdleInTxSessionTimeout: models.MustNewDuration(time.Minute), @@ -666,7 +667,6 @@ func TestConfig_Marshal(t *testing.T) { }, }, } - full.TxmAsService = toml.TxmAsService{Enabled: ptr(true)} for _, tt := range []struct { name string @@ -704,6 +704,7 @@ Headers = ['Authorization: token', 'X-SomeOther-Header: value with spaces | and FeedsManager = true LogPoller = true UICSAKeys = true +TransactionService = true `}, {"Database", Config{Core: toml.Core{Database: full.Database}}, `[Database] DefaultIdleInTxSessionTimeout = '1m0s' diff --git a/core/services/chainlink/config_txm_as_service.go b/core/services/chainlink/config_txm_as_service.go deleted file mode 100644 index 50e1fe381d9..00000000000 --- a/core/services/chainlink/config_txm_as_service.go +++ /dev/null @@ -1,16 +0,0 @@ -package chainlink - -import ( - "github.com/smartcontractkit/chainlink/v2/core/config" - "github.com/smartcontractkit/chainlink/v2/core/config/toml" -) - -var _ config.TxmAsService = (*txmAsServiceConfig)(nil) - -type txmAsServiceConfig struct { - c toml.TxmAsService -} - -func (t *txmAsServiceConfig) Enabled() bool { - return t.c.Enabled != nil && *t.c.Enabled -} diff --git a/core/services/chainlink/mocks/general_config.go b/core/services/chainlink/mocks/general_config.go index 4592cc218b6..98796e90053 100644 --- a/core/services/chainlink/mocks/general_config.go +++ b/core/services/chainlink/mocks/general_config.go @@ -591,22 +591,6 @@ func (_m *GeneralConfig) Tracing() config.Tracing { return r0 } -// TxmAsService provides a mock function with given fields: -func (_m *GeneralConfig) TxmAsService() config.TxmAsService { - ret := _m.Called() - - var r0 config.TxmAsService - if rf, ok := ret.Get(0).(func() config.TxmAsService); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(config.TxmAsService) - } - } - - return r0 -} - // Validate provides a mock function with given fields: func (_m *GeneralConfig) Validate() error { ret := _m.Called() diff --git a/core/services/chainlink/testdata/config-empty-effective.toml b/core/services/chainlink/testdata/config-empty-effective.toml index 55edfd0cf01..16740aa12b0 100644 --- a/core/services/chainlink/testdata/config-empty-effective.toml +++ b/core/services/chainlink/testdata/config-empty-effective.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -232,6 +233,3 @@ Enabled = false CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 - -[TxmAsService] -Enabled = false diff --git a/core/services/chainlink/testdata/config-full.toml b/core/services/chainlink/testdata/config-full.toml index 7053153e0bc..8e4d4130b79 100644 --- a/core/services/chainlink/testdata/config-full.toml +++ b/core/services/chainlink/testdata/config-full.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '10s' FeedsManager = true LogPoller = true UICSAKeys = true +TransactionService = true [Database] DefaultIdleInTxSessionTimeout = '1m0s' @@ -243,9 +244,6 @@ SamplingRatio = 1.0 env = 'dev' test = 'load' -[TxmAsService] -Enabled = true - [[EVM]] ChainID = '1' Enabled = false diff --git a/core/services/chainlink/testdata/config-multi-chain-effective.toml b/core/services/chainlink/testdata/config-multi-chain-effective.toml index 853e393d58c..5b7d00433af 100644 --- a/core/services/chainlink/testdata/config-multi-chain-effective.toml +++ b/core/services/chainlink/testdata/config-multi-chain-effective.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -233,9 +234,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/core/web/evm_transactions_controller.go b/core/web/evm_transactions_controller.go index ede7acd07be..7e21e6295e8 100644 --- a/core/web/evm_transactions_controller.go +++ b/core/web/evm_transactions_controller.go @@ -78,7 +78,7 @@ type EvmTransactionController struct { func NewEVMTransactionController(app chainlink.Application) *EvmTransactionController { return &EvmTransactionController{ - Enabled: app.GetConfig().TxmAsService().Enabled(), + Enabled: app.GetConfig().Feature().TransactionService(), Logger: app.GetLogger(), AuditLogger: app.GetAuditLogger(), Chains: app.GetRelayers().LegacyEVMChains(), @@ -90,7 +90,8 @@ func NewEVMTransactionController(app chainlink.Application) *EvmTransactionContr // specified chain. func (tc *EvmTransactionController) Create(c *gin.Context) { if !tc.Enabled { - jsonAPIError(c, http.StatusUnprocessableEntity, errors.New("transactions creation disabled. To enable set TxmAsService.Enabled=true")) + jsonAPIError(c, http.StatusUnprocessableEntity, + errors.New("transactions creation disabled. To enable set Feature.TransactionService=true")) return } var tx models.CreateEVMTransactionRequest diff --git a/core/web/evm_transactions_controller_test.go b/core/web/evm_transactions_controller_test.go index 5304209b181..9c3549fb488 100644 --- a/core/web/evm_transactions_controller_test.go +++ b/core/web/evm_transactions_controller_test.go @@ -207,7 +207,7 @@ func TestTransactionsController_Create(t *testing.T) { router.ServeHTTP(resp, req) cltest.AssertServerResponse(t, resp.Result(), http.StatusUnprocessableEntity) respError := cltest.ParseJSONAPIErrors(t, resp.Body) - require.Equal(t, "transactions creation disabled. To enable set TxmAsService.Enabled=true", respError.Error()) + require.Equal(t, "transactions creation disabled. To enable set Feature.TransactionService=true", respError.Error()) }) createTx := func(controller *web.EvmTransactionController, request interface{}) *httptest.ResponseRecorder { diff --git a/core/web/resolver/testdata/config-empty-effective.toml b/core/web/resolver/testdata/config-empty-effective.toml index 55edfd0cf01..16740aa12b0 100644 --- a/core/web/resolver/testdata/config-empty-effective.toml +++ b/core/web/resolver/testdata/config-empty-effective.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -232,6 +233,3 @@ Enabled = false CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 - -[TxmAsService] -Enabled = false diff --git a/core/web/resolver/testdata/config-full.toml b/core/web/resolver/testdata/config-full.toml index 3d5adbbc049..4ed736e1588 100644 --- a/core/web/resolver/testdata/config-full.toml +++ b/core/web/resolver/testdata/config-full.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '10s' FeedsManager = true LogPoller = true UICSAKeys = true +TransactionService = true [Database] DefaultIdleInTxSessionTimeout = '1m0s' @@ -243,9 +244,6 @@ SamplingRatio = 1.0 env = 'dev' test = 'load' -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' Enabled = false diff --git a/core/web/resolver/testdata/config-multi-chain-effective.toml b/core/web/resolver/testdata/config-multi-chain-effective.toml index 853e393d58c..5b7d00433af 100644 --- a/core/web/resolver/testdata/config-multi-chain-effective.toml +++ b/core/web/resolver/testdata/config-multi-chain-effective.toml @@ -6,6 +6,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -233,9 +234,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 55e4c84348a..0d3211b1b2d 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -51,6 +51,7 @@ ShutdownGracePeriod is the maximum time allowed to shut down gracefully. If exce FeedsManager = true # Default LogPoller = false # Default UICSAKeys = false # Default +TransactionService = false # Default ``` @@ -72,6 +73,12 @@ UICSAKeys = false # Default ``` UICSAKeys enables CSA Keys in the UI. +### TransactionService +```toml +TransactionService = false # Default +``` +TransactionService enables `POST /v2/transactions/evm` endpoint. + ## Database ```toml [Database] @@ -1638,20 +1645,6 @@ env = "test" # Example ``` env is an example user specified key-value pair -## TxmAsService -```toml -[TxmAsService] -Enabled = false # Default -``` - - -### Enabled -```toml -Enabled = false # Default -``` -Enabled turns Transaction Manager as a service feature on or off. When enabled exposes endpoint to submit arbitrary EVM transaction -using one of the keys enabled for the specified chain. - ## EVM EVM defaults depend on ChainID: diff --git a/integration-tests/types/config/node/core.go b/integration-tests/types/config/node/core.go index 7ddddafdd5f..6d7cb2469c0 100644 --- a/integration-tests/types/config/node/core.go +++ b/integration-tests/types/config/node/core.go @@ -13,6 +13,7 @@ import ( relayassets "github.com/smartcontractkit/chainlink-relay/pkg/assets" "github.com/smartcontractkit/chainlink-testing-framework/blockchain" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" evmcfg "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml" "github.com/smartcontractkit/chainlink/v2/core/config/toml" diff --git a/testdata/scripts/node/validate/default.txtar b/testdata/scripts/node/validate/default.txtar index d7ef0ac7bbb..64bc5ed614f 100644 --- a/testdata/scripts/node/validate/default.txtar +++ b/testdata/scripts/node/validate/default.txtar @@ -18,6 +18,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -245,9 +246,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - Invalid configuration: invalid secrets: 2 errors: - Database.URL: empty: must be provided and non-empty - Password.Keystore: empty: must be provided and non-empty diff --git a/testdata/scripts/node/validate/disk-based-logging-disabled.txtar b/testdata/scripts/node/validate/disk-based-logging-disabled.txtar index 0dd9c2e8a17..8a0e60bbb78 100644 --- a/testdata/scripts/node/validate/disk-based-logging-disabled.txtar +++ b/testdata/scripts/node/validate/disk-based-logging-disabled.txtar @@ -62,6 +62,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -289,9 +290,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar b/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar index 3939df994e5..1de3bec4945 100644 --- a/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar +++ b/testdata/scripts/node/validate/disk-based-logging-no-dir.txtar @@ -62,6 +62,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -289,9 +290,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/disk-based-logging.txtar b/testdata/scripts/node/validate/disk-based-logging.txtar index bf7b1e0c412..ae19706cac1 100644 --- a/testdata/scripts/node/validate/disk-based-logging.txtar +++ b/testdata/scripts/node/validate/disk-based-logging.txtar @@ -62,6 +62,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -289,9 +290,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/invalid.txtar b/testdata/scripts/node/validate/invalid.txtar index 389f98a60b5..23beed51b2a 100644 --- a/testdata/scripts/node/validate/invalid.txtar +++ b/testdata/scripts/node/validate/invalid.txtar @@ -52,6 +52,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -279,9 +280,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/valid.txtar b/testdata/scripts/node/validate/valid.txtar index d1f3230be8b..365e7839278 100644 --- a/testdata/scripts/node/validate/valid.txtar +++ b/testdata/scripts/node/validate/valid.txtar @@ -59,6 +59,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -286,9 +287,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - [[EVM]] ChainID = '1' AutoCreateKey = true diff --git a/testdata/scripts/node/validate/warnings.txtar b/testdata/scripts/node/validate/warnings.txtar index 8fa48027607..786d9cff4e5 100644 --- a/testdata/scripts/node/validate/warnings.txtar +++ b/testdata/scripts/node/validate/warnings.txtar @@ -55,6 +55,7 @@ ShutdownGracePeriod = '5s' FeedsManager = true LogPoller = false UICSAKeys = false +TransactionService = false [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' @@ -282,9 +283,6 @@ CollectorTarget = '' NodeID = '' SamplingRatio = 0.0 -[TxmAsService] -Enabled = false - # Configuration warning: 2 errors: - P2P.V1: is deprecated and will be removed in a future version From 2d52d709adeae194d0a4d3b55e4f41c25b8be83d Mon Sep 17 00:00:00 2001 From: Dmytro Haidashenko Date: Wed, 29 Nov 2023 14:32:49 +0100 Subject: [PATCH 16/16] switch to sqlx.REbind for dynamic sql query --- core/chains/evm/txmgr/evm_tx_store.go | 30 +++++++++++---------------- core/scripts/go.mod | 3 --- core/scripts/go.sum | 6 ------ go.mod | 6 ------ go.sum | 6 ------ integration-tests/go.mod | 3 --- integration-tests/go.sum | 6 ------ 7 files changed, 12 insertions(+), 48 deletions(-) diff --git a/core/chains/evm/txmgr/evm_tx_store.go b/core/chains/evm/txmgr/evm_tx_store.go index 550b15cd7bf..c0bf94c64df 100644 --- a/core/chains/evm/txmgr/evm_tx_store.go +++ b/core/chains/evm/txmgr/evm_tx_store.go @@ -10,7 +10,6 @@ import ( "strings" "time" - sq "github.com/Masterminds/squirrel" "github.com/ethereum/go-ethereum/common" "github.com/google/uuid" "github.com/jackc/pgconn" @@ -20,6 +19,7 @@ import ( nullv4 "gopkg.in/guregu/null.v4" "github.com/smartcontractkit/chainlink-common/pkg/sqlutil" + "github.com/smartcontractkit/chainlink/v2/common/txmgr" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" @@ -450,29 +450,23 @@ type TransactionsWithAttemptsSelector struct { // TransactionsWithAttempts returns all eth transactions with at least one attempt // limited by passed parameters. Attempts are sorted by id. func (o *evmTxStore) TransactionsWithAttempts(selector TransactionsWithAttemptsSelector) (txs []Tx, count int, err error) { - stmt := sq.Select("count(*)").From("evm.txes"). - Where("id IN (SELECT DISTINCT eth_tx_id FROM evm.tx_attempts)").PlaceholderFormat(sq.Dollar) + query := " FROM evm.txes WHERE id IN (SELECT DISTINCT eth_tx_id FROM evm.tx_attempts)" + var args []interface{} if selector.IdempotencyKey != nil { - stmt = stmt.Where("idempotency_key = ?", *selector.IdempotencyKey) - } - - query, args, err := stmt.ToSql() - if err != nil { - return nil, 0, fmt.Errorf("failed to build count query: %w", err) + args = append(args, *selector.IdempotencyKey) + query += " AND idempotency_key = ?" } - if err = o.q.Get(&count, query, args...); err != nil { - return nil, 0, fmt.Errorf("failed to exec count query: %w, sql: %s", err, query) + countQuery := sqlx.Rebind(sqlx.DOLLAR, "SELECT count(*)"+query) + if err = o.q.Get(&count, countQuery, args...); err != nil { + return nil, 0, fmt.Errorf("failed to exec count query: %w, sql: %s", err, countQuery) } - query, args, err = stmt.RemoveColumns().Columns("*"). - OrderBy("id desc").Limit(selector.Limit).Offset(selector.Offset).ToSql() - if err != nil { - return nil, 0, fmt.Errorf("failed to build select query: %w", err) - } + selectQuery := sqlx.Rebind(sqlx.DOLLAR, "SELECT * "+query+" ORDER BY id desc LIMIT ? OFFSET ?") + args = append(args, selector.Limit, selector.Offset) var dbTxs []DbEthTx - if err = o.q.Select(&dbTxs, query, args...); err != nil { - return nil, 0, fmt.Errorf("failed to exec select query: %w, sql: %s", err, query) + if err = o.q.Select(&dbTxs, selectQuery, args...); err != nil { + return nil, 0, fmt.Errorf("failed to exec select query: %w, sql: %s", err, selectQuery) } txs = dbEthTxsToEvmEthTxs(dbTxs) err = o.preloadTxAttempts(txs) diff --git a/core/scripts/go.mod b/core/scripts/go.mod index 2466934789b..e9fa432d4cd 100644 --- a/core/scripts/go.mod +++ b/core/scripts/go.mod @@ -51,7 +51,6 @@ require ( github.com/DataDog/zstd v1.5.2 // indirect github.com/Depado/ginprom v1.7.11 // indirect github.com/Masterminds/semver/v3 v3.2.1 // indirect - github.com/Masterminds/squirrel v1.5.4 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect github.com/VictoriaMetrics/fastcache v1.10.0 // indirect github.com/andres-erbsen/clock v0.0.0-20160526145045-9e14626cd129 // indirect @@ -208,8 +207,6 @@ require ( github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect - github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect - github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/lib/pq v1.10.9 // indirect diff --git a/core/scripts/go.sum b/core/scripts/go.sum index 6053cddee9c..3d9962474b9 100644 --- a/core/scripts/go.sum +++ b/core/scripts/go.sum @@ -105,8 +105,6 @@ github.com/Kubuxu/go-os-helper v0.0.1/go.mod h1:N8B+I7vPCT80IcP58r50u4+gEEcsZETF github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= -github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= -github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= @@ -921,10 +919,6 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= diff --git a/go.mod b/go.mod index 601710029f1..e6bb7347ffd 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/Depado/ginprom v1.7.11 github.com/Masterminds/semver/v3 v3.2.1 github.com/Masterminds/sprig/v3 v3.2.3 - github.com/Masterminds/squirrel v1.5.4 github.com/avast/retry-go/v4 v4.5.0 github.com/btcsuite/btcd v0.23.4 github.com/cometbft/cometbft v0.37.2 @@ -104,11 +103,6 @@ require ( gopkg.in/natefinch/lumberjack.v2 v2.0.0 ) -require ( - github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect - github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect -) - require ( contrib.go.opencensus.io/exporter/stackdriver v0.13.5 // indirect cosmossdk.io/api v0.3.1 // indirect diff --git a/go.sum b/go.sum index ef7ead42699..24d662390d0 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,6 @@ github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0 github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= github.com/Masterminds/sprig/v3 v3.2.3 h1:eL2fZNezLomi0uOLqjQoN6BfsDD+fyLtgbJMAj9n6YA= github.com/Masterminds/sprig/v3 v3.2.3/go.mod h1:rXcFaZ2zZbLRJv/xSysmlgIM1u11eBaRMhvYXJNkGuM= -github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= -github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= @@ -922,10 +920,6 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= diff --git a/integration-tests/go.mod b/integration-tests/go.mod index 943616118df..ba90493d299 100644 --- a/integration-tests/go.mod +++ b/integration-tests/go.mod @@ -66,7 +66,6 @@ require ( github.com/K-Phoen/sdk v0.12.2 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect github.com/Masterminds/semver/v3 v3.2.1 // indirect - github.com/Masterminds/squirrel v1.5.4 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect github.com/VictoriaMetrics/fastcache v1.10.0 // indirect github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect @@ -271,8 +270,6 @@ require ( github.com/koron/go-ssdp v0.0.2 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect - github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect - github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/libp2p/go-addr-util v0.0.2 // indirect diff --git a/integration-tests/go.sum b/integration-tests/go.sum index b09232a11f3..849df2afcab 100644 --- a/integration-tests/go.sum +++ b/integration-tests/go.sum @@ -610,8 +610,6 @@ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6 github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= -github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= -github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= github.com/Microsoft/hcsshim v0.10.0-rc.8 h1:YSZVvlIIDD1UxQpJp0h+dnpLUw+TrY0cx8obKsp3bek= @@ -1712,10 +1710,6 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.5.0/go.mod h1:czIriw4a0C1dFun+ObrXp7ok03xON0N1awStJ6ArI7Y= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= -github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk= -github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a h1:dHCfT5W7gghzPtfsW488uPmEOm85wewI+ypUwibyTdU= github.com/leanovate/gopter v0.2.10-0.20210127095200-9abe2343507a/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII=