Fix/pigeons attest rolledback txs (#1284)

# Related Github tickets

- VolumeFi#2095

# Background

Paloma was attesting to pigeon relayed messages using only the
transaction, but not the receipt, meaning it couldn't actually attest
the transaction was successful. This (and a corresponding PR in pigeon)
makes paloma verify the transaction result from the transaction receipt
and only attest to successful transactions.

Validated with an SLC message in the private testnet using Gnosis, but
the result should be the same for all turnstone messages. This wasn't
replicable in Arbitrum, where messages fail immediately on pigeons.

Also in this PR is a fix for SLC and User Smart Contract Upload retries.
We need to clear the Fees from the message before a retry, otherwise we
get an Invalid Signature error.

# Testing completed

- [x] test coverage exists or has been added/updated
- [x] tested in a private testnet

# Breaking changes

- [x] I have checked my code for breaking changes
- [x] If there are breaking changes, there is a supporting migration.

x/evm/keeper/attest.go

@@ -45,7 +45,8 @@ func (k Keeper) attestMessageWrapper(ctx context.Context, q consensus.Queuer, ms
 	defer func() {
 		// If there is no error, or we can't verify this transaction, we flush
 		// the context, so we never see it again
-		if retErr == nil || errors.Is(retErr, types.ErrEthTxNotVerified) {
+		if retErr == nil || errors.Is(retErr, types.ErrEthTxNotVerified) ||
+			errors.Is(retErr, types.ErrEthTxFailed) {
 			writeCache()
 		}
 	}()
@@ -122,6 +123,25 @@ func (k Keeper) routerAttester(sdkCtx sdk.Context, q consensus.Queuer, msg conse
 		rawEvidence:      winner,
 		msg:              message,
 	}
+
+	switch winner := winner.(type) {
+	case *types.TxExecutedProof:
+		// If we have proof of a transaction, we need to check the status on
+		// the receipt
+		receipt, err := winner.GetReceipt()
+		if err != nil {
+			logger.WithFields("message-id", msg.GetId()).
+				WithError(err).Error("Failed to get transaction receipt")
+			return err
+		}
+
+		if receipt.Status != ethtypes.ReceiptStatusSuccessful {
+			logger.WithFields("message-id", msg.GetId(), "status", receipt.Status).
+				Warn("Transaction execution failed")
+			return types.ErrEthTxFailed
+		}
+	}
+
 	switch rawAction.(type) {
 	case *types.Message_UploadSmartContract:
 		return newUploadSmartContractAttester(&k, logger, params).Execute(sdkCtx)

x/evm/keeper/attest_compass_handover_test.go

@@ -162,6 +162,7 @@ var _ = Describe("attest compass handover", func() {
 			Expect(err).To(BeNil())
 
 			receipt := ethcoretypes.Receipt{
+				Status: ethcoretypes.ReceiptStatusSuccessful,
 				Logs: []*ethcoretypes.Log{
 					{
 						Topics: []common.Hash{contractDeployedEvent},

x/evm/keeper/attest_submit_logic_call.go

@@ -65,6 +65,8 @@ func (a *submitLogicCallAttester) attemptRetry(ctx sdk.Context, proof *types.Sma
 	slc := a.action
 	if slc.Retries < cMaxSubmitLogicCallRetries {
 		slc.Retries++
+		// We must clear fees before retry or the signature verification fails
+		slc.Fees = nil
 		a.logger.Info("retrying failed SubmitLogicCall message",
 			"message-id", a.msgID,
 			"retries", slc.Retries,

x/evm/keeper/attest_test.go

@@ -49,6 +49,10 @@ var (
 	valsetTx1 = ethcoretypes.NewTx(&ethcoretypes.DynamicFeeTx{
 		Data: common.FromHex(string(whoops.Must(os.ReadFile("testdata/valset-tx-data.hex")))),
 	})
+
+	serializedReceipt, _ = (&ethcoretypes.Receipt{
+		Status: ethcoretypes.ReceiptStatusSuccessful,
+	}).MarshalBinary()
 )
 
 type record struct {
@@ -359,7 +363,10 @@ var _ = g.Describe("attest router", func() {
 				g.When("message is SubmitLogicCall", func() {
 					g.BeforeEach(func() {
 						execTx = slcTx1
-						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{SerializedTX: whoops.Must(execTx.MarshalBinary())}))
+						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{
+							SerializedTX:      whoops.Must(execTx.MarshalBinary()),
+							SerializedReceipt: serializedReceipt,
+						}))
 						evidence = []*consensustypes.Evidence{
 							{
 								ValAddress: sdk.ValAddress("validator-1"),
@@ -445,7 +452,10 @@ var _ = g.Describe("attest router", func() {
 				g.When("message is UpdateValset", func() {
 					g.BeforeEach(func() {
 						execTx = valsetTx1
-						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{SerializedTX: whoops.Must(execTx.MarshalBinary())}))
+						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{
+							SerializedTX:      whoops.Must(execTx.MarshalBinary()),
+							SerializedReceipt: serializedReceipt,
+						}))
 						evidence = []*consensustypes.Evidence{
 							{
 								ValAddress: sdk.ValAddress("validator-1"),
@@ -494,7 +504,10 @@ var _ = g.Describe("attest router", func() {
 				g.When("message is UploadSmartContract", func() {
 					g.BeforeEach(func() {
 						execTx = uscTx1
-						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{SerializedTX: whoops.Must(execTx.MarshalBinary())}))
+						proof := whoops.Must(codectypes.NewAnyWithValue(&types.TxExecutedProof{
+							SerializedTX:      whoops.Must(execTx.MarshalBinary()),
+							SerializedReceipt: serializedReceipt,
+						}))
 						evidence = []*consensustypes.Evidence{
 							{
 								ValAddress: sdk.ValAddress("validator-1"),

x/evm/keeper/attest_upload_user_smart_contract.go

@@ -147,6 +147,8 @@ func (a *uploadUserSmartContractAttester) attemptRetry(ctx sdk.Context) {
 	}
 
 	a.action.Retries++
+	// We must clear fees before retry or the signature verification fails
+	a.action.Fees = nil
 
 	a.logger.Info("Retrying failed UploadUserSmartContract message",
 		"message-id", a.msgID,

x/evm/keeper/attest_upload_user_smart_contract_test.go

@@ -162,6 +162,7 @@ var _ = Describe("attest upload user smart contract", func() {
 			Expect(err).To(BeNil())
 
 			receipt := ethcoretypes.Receipt{
+				Status: ethcoretypes.ReceiptStatusSuccessful,
 				Logs: []*ethcoretypes.Log{
 					{
 						Topics: []common.Hash{contractDeployedEvent},

x/evm/types/errors.go

@@ -15,5 +15,6 @@ var (
 
 var (
 	ErrEthTxNotVerified = whoops.String("transaction not verified")
+	ErrEthTxFailed      = whoops.String("transaction failed to execute")
 	ErrInvalidBalance   = whoops.Errorf("invalid balance: %s")
 )