Skip to content

Commit

Permalink
return err and status for txm
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xnogo
0xnogo committed Sep 2, 2024
1 parent a793f34 commit fa6d9c3
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 39 deletions.
13 changes: 8 additions & 5 deletions core/services/ocr2/plugins/ccip/ccipexec/batching.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (bs ZKOverflowBatchingStrategy) BuildBatch(
// Check if msg is inflight
if exists := inflightSeqNums.Contains(msg.SequenceNumber); exists {
// Message is inflight, skip it
msgLggr.Infow("Skipping message - already inflight", "message", msgId)
msgLggr.Infow("Skipping message - already inflight")
batchBuilder.skip(msg, SkippedInflight)
continue
}
Expand All @@ -143,13 +143,16 @@ func (bs ZKOverflowBatchingStrategy) BuildBatch(

if len(statuses) == 0 {
// No status found for message = first time we see it
msgLggr.Infow("No status found for message - proceeding with checks", "message", msgId)
msgLggr.Infow("No status found for message - proceeding with checks")
} else {
// Status(es) found for message = check if any of them is final to decide if we should add it to the batch
hasFatalStatus := false
for _, s := range statuses {
if s == types.Fatal {
msgLggr.Infow("Skipping message - found a fatal TXM status", "message", msgId)
if s.Status == types.Fatal || s.Status == types.Failed {
msgLggr.Warnw("TXM error status found", "status", s.Status, "error", s.Error)
}
if s.Status == types.Fatal {
msgLggr.Infow("Skipping message - found a fatal TXM status", "status", s.Status)
batchBuilder.skip(msg, TXMFatalStatus)
hasFatalStatus = true
break
Expand All @@ -158,7 +161,7 @@ func (bs ZKOverflowBatchingStrategy) BuildBatch(
if hasFatalStatus {
continue
}
msgLggr.Infow("No fatal status found for message - proceeding with checks", "message", msgId)
msgLggr.Infow("No fatal status found for message - proceeding with checks")
}

status, messageMaxGas, tokenData, msgValue, err := performCommonChecks(ctx, batchCtx, msg, msgLggr)
Expand Down
39 changes: 24 additions & 15 deletions core/services/ocr2/plugins/ccip/ccipexec/batching_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcalc"
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/prices"
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/tokendata"
"github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/statuschecker"
mockstatuschecker "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/statuschecker/mocks"
)

Expand Down Expand Up @@ -579,7 +580,7 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, mock.Anything).Return([]types.TransactionStatus{}, -1, nil)
m.On("CheckMessageStatus", mock.Anything, mock.Anything).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
},
},
{
Expand All @@ -597,7 +598,7 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Fatal}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{{Status: types.Fatal, Error: errors.New("dummy")}}, 0, nil)
},
skipGasPriceEstimator: true,
},
Expand All @@ -618,8 +619,8 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Fatal}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]types.TransactionStatus{}, -1, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{{Status: types.Fatal, Error: errors.New("dummy error")}}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
},
},
{
Expand All @@ -638,8 +639,8 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Fatal}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]types.TransactionStatus{types.Fatal}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{{Status: types.Fatal, Error: errors.New("dummy error")}}, 0, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{{Status: types.Fatal, Error: errors.New("dummy error")}}, 0, nil)
},
skipGasPriceEstimator: true,
},
Expand All @@ -659,7 +660,7 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Unconfirmed, types.Failed}, 1, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{{Status: types.Unknown, Error: nil}, {Status: types.Failed, Error: errors.New("dummy error")}}, 1, nil)
},
},
{
Expand All @@ -679,8 +680,12 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Unconfirmed, types.Failed, types.Fatal}, 2, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]types.TransactionStatus{}, -1, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{
{Status: types.Unconfirmed, Error: nil},
{Status: types.Failed, Error: errors.New("dummy error")},
{Status: types.Fatal, Error: errors.New("dummy error")},
}, 2, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
},
},
{
Expand All @@ -700,8 +705,8 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{}, -1, errors.New("dummy txm error"))
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]types.TransactionStatus{}, -1, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, errors.New("dummy txm error"))
m.On("CheckMessageStatus", mock.Anything, zkMsg2.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
},
},
{
Expand Down Expand Up @@ -734,7 +739,7 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{}, -1, errors.New("dummy txm error"))
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, errors.New("dummy txm error"))
},
skipGasPriceEstimator: true,
},
Expand All @@ -753,7 +758,11 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]types.TransactionStatus{types.Unconfirmed, types.Failed, types.Fatal}, 2, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg1.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{
{Status: types.Unconfirmed, Error: nil},
{Status: types.Failed, Error: errors.New("dummy error")},
{Status: types.Fatal, Error: errors.New("dummy error")},
}, 2, nil)
},
skipGasPriceEstimator: true,
},
Expand All @@ -775,7 +784,7 @@ func TestBatchingStrategies(t *testing.T) {
},
statuschecker: func(m *mockstatuschecker.CCIPTransactionStatusChecker) {
m.Mock = mock.Mock{} // reset mock
m.On("CheckMessageStatus", mock.Anything, zkMsg3.MessageID.String()).Return([]types.TransactionStatus{}, -1, nil)
m.On("CheckMessageStatus", mock.Anything, zkMsg3.MessageID.String()).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
},
skipGasPriceEstimator: false,
},
Expand Down Expand Up @@ -815,7 +824,7 @@ func runBatchingStrategyTests(t *testing.T, strategy BatchingStrategy, available

// default case for ZKOverflowBatchingStrategy
if strategyType := reflect.TypeOf(strategy); tc.statuschecker == nil && strategyType == reflect.TypeOf(&ZKOverflowBatchingStrategy{}) {
strategy.(*ZKOverflowBatchingStrategy).statuschecker.(*mockstatuschecker.CCIPTransactionStatusChecker).On("CheckMessageStatus", mock.Anything, mock.Anything).Return([]types.TransactionStatus{}, -1, nil)
strategy.(*ZKOverflowBatchingStrategy).statuschecker.(*mockstatuschecker.CCIPTransactionStatusChecker).On("CheckMessageStatus", mock.Anything, mock.Anything).Return([]statuschecker.TransactionStatusWithError{}, -1, nil)
}

// Mock calls to TXM
Expand Down
17 changes: 8 additions & 9 deletions core/services/relay/evm/statuschecker/mocks/ccip_transaction_status_checker.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 15 additions & 4 deletions core/services/relay/evm/statuschecker/txm_status_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ import (
// It returns a list of transaction statuses, the retry counter, and an error if any occurred during the process.
//

type TransactionStatusWithError struct {
Status types.TransactionStatus
Error error
}

type CCIPTransactionStatusChecker interface {
CheckMessageStatus(ctx context.Context, msgID string) (transactionStatuses []types.TransactionStatus, retryCounter int, err error)
CheckMessageStatus(ctx context.Context, msgID string) (transactionStatuses []TransactionStatusWithError, retryCounter int, err error)
}

type TxmStatusChecker struct {
Expand All @@ -28,20 +33,26 @@ func NewTxmStatusChecker(getTransactionStatus func(ctx context.Context, transact
// It returns a slice of all statuses and the number of transactions found (-1 if none).
// The key will follow the format: <msgID>-<counter>. TXM will be queried for each key until a NotFound error is returned.
// The goal is to find all transactions associated with a message ID and snooze messages if they are fatal in the Execution Plugin.
func (tsc *TxmStatusChecker) CheckMessageStatus(ctx context.Context, msgID string) ([]types.TransactionStatus, int, error) {
func (tsc *TxmStatusChecker) CheckMessageStatus(ctx context.Context, msgID string) ([]TransactionStatusWithError, int, error) {
var counter int
const maxStatuses = 1000 // Cap the number of statuses to avoid infinite loop

allStatuses := make([]types.TransactionStatus, 0)
allStatuses := make([]TransactionStatusWithError, 0)

for {
transactionID := fmt.Sprintf("%s-%d", msgID, counter)
status, err := tsc.getTransactionStatus(ctx, transactionID)

statusWithError := TransactionStatusWithError{
Status: status,
Error: err,
}

if err != nil && status == types.Unknown {
// If the status is unknown and err not nil, it means the transaction was not found
break
}
allStatuses = append(allStatuses, status)
allStatuses = append(allStatuses, statusWithError)
counter++

// Break the loop if the cap is reached
Expand Down
17 changes: 11 additions & 6 deletions core/services/relay/evm/statuschecker/txm_status_checker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func Test_CheckMessageStatus(t *testing.T) {
testCases := []struct {
name string
setupMock func()
expectedStatus []types.TransactionStatus
expectedStatus []TransactionStatusWithError
expectedCounter int
expectedError error
}{
Expand All @@ -36,7 +36,7 @@ func Test_CheckMessageStatus(t *testing.T) {
mockTxManager.Mock = mock.Mock{}
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-0").Return(types.Unknown, errors.New("failed to find transaction with IdempotencyKey test-message-id-0"))
},
expectedStatus: []types.TransactionStatus{},
expectedStatus: []TransactionStatusWithError{},
expectedCounter: -1,
expectedError: nil,
},
Expand All @@ -47,7 +47,7 @@ func Test_CheckMessageStatus(t *testing.T) {
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-0").Return(types.Finalized, nil)
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-1").Return(types.Unknown, errors.New("failed to find transaction with IdempotencyKey test-message-id-1"))
},
expectedStatus: []types.TransactionStatus{types.Finalized},
expectedStatus: []TransactionStatusWithError{{Status: types.Finalized, Error: nil}},
expectedCounter: 0,
expectedError: nil,
},
Expand All @@ -56,10 +56,13 @@ func Test_CheckMessageStatus(t *testing.T) {
setupMock: func() {
mockTxManager.Mock = mock.Mock{}
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-0").Return(types.Finalized, nil)
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-1").Return(types.Failed, nil)
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-1").Return(types.Failed, errors.New("dummy error"))
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-2").Return(types.Unknown, errors.New("failed to find transaction with IdempotencyKey test-message-id-2"))
},
expectedStatus: []types.TransactionStatus{types.Finalized, types.Failed},
expectedStatus: []TransactionStatusWithError{
{Status: types.Finalized, Error: nil},
{Status: types.Failed, Error: errors.New("dummy error")},
},
expectedCounter: 1,
expectedError: nil,
},
Expand All @@ -70,7 +73,9 @@ func Test_CheckMessageStatus(t *testing.T) {
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-0").Return(types.Unknown, nil)
mockTxManager.On("GetTransactionStatus", ctx, "test-message-id-1").Return(types.Unknown, errors.New("failed to find transaction with IdempotencyKey test-message-id-1"))
},
expectedStatus: []types.TransactionStatus{types.Unknown},
expectedStatus: []TransactionStatusWithError{
{Status: types.Unknown, Error: nil},
},
expectedCounter: 0,
expectedError: nil,
},
Expand Down

0 comments on commit fa6d9c3

Please sign in to comment.