Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set ErrorData on all errors from handling a turnstone message #439

Merged
merged 3 commits into from
Sep 20, 2024
Merged

fix: set ErrorData on all errors from handling a turnstone message #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04ec000
chore: return all compass call errors
maharifu Sep 20, 2024
28bd68c
chore: return compass errors processing messages
maharifu Sep 20, 2024
bb3a742
chore: cleanup error handling
maharifu Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 26 additions & 73 deletions chain/evm/compass.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,18 +188,7 @@ func (t compass) updateValset(
estimate,
})
if err != nil {
logger.WithError(err).Error("call_compass error")
if opts.estimateOnly == false {
isSmartContractError, setErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err)
if setErr != nil {
return nil, 0, setErr
}
if isSmartContractError {
logger.Debug("smart contract error. recovering...")
return nil, 0, nil
}
}
return nil, 0, fmt.Errorf("call_compass error: %w", err)
return nil, 0, err
}

return tx, currentValsetID, nil
Expand Down Expand Up @@ -281,18 +270,6 @@ func (t compass) submitLogicCall(
logger.WithField("consensus", con).WithField("args", args).Debug("submitting logic call")
tx, err := t.callCompass(ctx, opts, "submit_logic_call", args)
if err != nil {
logger.WithError(err).Error("submitLogicCall: error calling DeployContract")
if opts.estimateOnly == false {
isSmartContractError, setErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err)
if setErr != nil {
return nil, 0, setErr
}
if isSmartContractError {
logger.Debug("smart contract error. recovering...")
return nil, 0, nil
}
}

return nil, 0, err
}

Expand Down Expand Up @@ -359,18 +336,6 @@ func (t compass) compass_handover(
logger.WithField("consensus", con).WithField("args", args).Debug("compass handover")
tx, err := t.callCompass(ctx, opts, "compass_update_batch", args)
if err != nil {
logger.WithError(err).Error("CompassHandover: error calling DeployContract")
if opts.estimateOnly == false {
isSmartContractError, setErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err)
if setErr != nil {
return nil, 0, setErr
}
if isSmartContractError {
logger.Debug("smart contract error. recovering...")
return nil, 0, nil
}
}

return nil, 0, err
}

Expand Down Expand Up @@ -410,20 +375,6 @@ func (t compass) uploadSmartContract(
constructorInput,
)
if err != nil {
logger.
WithError(err).
WithField("input", constructorInput).
Error("uploadSmartContract: error calling DeployContract")

isSmartContractError, setErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err)
if setErr != nil {
return nil, setErr
}
if isSmartContractError {
logger.Debug("smart contract error. recovering...")
return nil, nil
}

return nil, err
}

Expand Down Expand Up @@ -501,29 +452,22 @@ func (t compass) uploadUserSmartContract(
Debug("deploying user smart contract")
tx, err := t.callCompass(ctx, opts, "deploy_contract", args)
if err != nil {
logger.WithError(err).Error("deploy_contract: error calling compass")
if opts.estimateOnly == false {
isSmartContractError, setErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err)
if setErr != nil {
return nil, 0, setErr
}
if isSmartContractError {
logger.Debug("smart contract error. recovering...")
return nil, 0, nil
}
}

return nil, 0, err
}

return tx, valsetID, nil
}

func (t compass) SetErrorData(ctx context.Context, queueTypeName string, msgID uint64, errToProcess error) (bool, error) {
func (t compass) SetErrorData(
ctx context.Context,
queueTypeName string,
msgID uint64,
errToProcess error,
) error {
data := []byte(errToProcess.Error())

var jsonRpcErr rpc.DataError
if !errors.As(errToProcess, &jsonRpcErr) {
return false, t.paloma.SetErrorData(ctx, queueTypeName, msgID, []byte(errToProcess.Error()))
} else {
if errors.As(errToProcess, &jsonRpcErr) {
liblog.WithContext(ctx).WithFields(
log.Fields{
"queue-type-name": queueTypeName,
Expand All @@ -532,13 +476,10 @@ func (t compass) SetErrorData(ctx context.Context, queueTypeName string, msgID u
},
).Warn("smart contract returned an error")

err := t.paloma.SetErrorData(ctx, queueTypeName, msgID, []byte(jsonRpcErr.Error()))
if err != nil {
return false, err
}

return true, nil
data = []byte(jsonRpcErr.Error())
}

return t.paloma.SetErrorData(ctx, queueTypeName, msgID, data)
}

func (t compass) findLastValsetMessageID(ctx context.Context) (uint64, error) {
Expand Down Expand Up @@ -877,6 +818,18 @@ func (t compass) processMessages(ctx context.Context, queueTypeName string, msgs
}
default:
logger.WithError(processingErr).Error("processing error")

if !opts.estimateOnly {
// If we're not just estimating, we want to set the error data
// on the message
setErr := t.SetErrorData(ctx, queueTypeName, rawMsg.ID, processingErr)
if setErr != nil {
// If we got an error setting the error data, this is the error
// we want to log
processingErr = setErr
}
}

if err := t.paloma.NewStatus().
WithChainReferenceID(t.ChainReferenceID).
WithQueueType(queueTypeName).
Expand Down Expand Up @@ -1719,7 +1672,7 @@ func (t compass) getFeeArgs(

if userFunds.Cmp(totalFundsNeeded) < 0 {
err := fmt.Errorf("insufficient funds for fees: %s < %s", userFunds, totalFundsNeeded)
if _, sendErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err); sendErr != nil {
if sendErr := t.SetErrorData(ctx, queueTypeName, origMessage.ID, err); sendErr != nil {
err = fmt.Errorf("failed to set error data: %w", sendErr)
}
return feeArgs, err
Expand Down
8 changes: 5 additions & 3 deletions chain/evm/compass_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func TestIsArbitraryCallAlreadyExecuted(t *testing.T) {

func TestMessageProcessing(t *testing.T) {
dummyErr := whoops.String("dummy")
rpcErr := fakeJsonRpcError("bla")

addValidSignature := func(pk *ecdsa.PrivateKey) chain.ValidatorSignature {
return signMessage(ethCompatibleBytesToSign, pk)
Expand Down Expand Up @@ -339,6 +340,7 @@ func TestMessageProcessing(t *testing.T) {
},
}
paloma.On("QueryGetSnapshotByID", mock.Anything, uint64(0)).Return(sn, nil)
paloma.On("SetErrorData", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)

return evm, paloma
},
Expand Down Expand Up @@ -1647,7 +1649,6 @@ func TestMessageProcessing(t *testing.T) {
},
setup: func(t *testing.T) (*mockEvmClienter, *evmmocks.PalomaClienter) {
evm, paloma := newMockEvmClienter(t), evmmocks.NewPalomaClienter(t)
fakeErr := fakeJsonRpcError("bla")

paloma.On("QueryGetEVMValsetByID", mock.Anything, uint64(0), "internal-chain-id").Return(
&types.Valset{
Expand All @@ -1657,11 +1658,12 @@ func TestMessageProcessing(t *testing.T) {
},
nil,
)
evm.On("DeployContract", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, fakeErr)
evm.On("DeployContract", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, rpcErr)
paloma.On("NewStatus").Return(&StatusUpdater{})
paloma.On("SetErrorData", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
return evm, paloma
},
expErr: nil,
expErr: rpcErr,
},
{
name: "upload_smart_contract/when smart contract returns an error and sending it to paloma fails, it returns it back",
Expand Down
Loading