Add panic-recover to safe utxo view connect txn (#1388)

lib/block_view_utils.go

@@ -1,6 +1,9 @@
 package lib
 
-import "github.com/pkg/errors"
+import (
+	"github.com/golang/glog"
+	"github.com/pkg/errors"
+)
 
 // SafeUtxoView is a wrapper around a UtxoView that provides a safe way to connect transactions
 // into a UtxoView without side effects when the connect fails.
@@ -40,6 +43,33 @@ func (safeUtxoView *SafeUtxoView) ConnectTransaction(
 	_fees uint64,
 	_err error,
 ) {
+	var revertBackupToPrimary bool
+	// If a transaction panics when connecting to the primary view, then
+	// we know it is invalid, and we should revert the primary view to the
+	// backup view. If the transaction panics when connecting to the backup
+	// view, then we should revert the backup view to the primary view and
+	// return a valid txn response.
+	// Note that we generally don't like to recover from panics as a panic
+	// signals an issue in the code that should be fixed. Additionally, we're
+	// breaking convention here as we don't like to set named return
+	// values, but we can't explicitly return values from a deferred function.
+	// We always prefer explicitly returning values instead of setting named
+	// return values as it makes the code easier to understand and maintain
+	// However, we are making an exception in this circumstance to ensure that
+	// nodes safely recover from a panic when trying to connect transactions.
+	defer func() {
+		if r := recover(); r != nil {
+			if revertBackupToPrimary {
+				safeUtxoView.backupView = safeUtxoView.primaryView.CopyUtxoView()
+				return
+			}
+			glog.Errorf("safeUtxoView.ConnectTransaction: Recovered from panic: %v", r)
+			_utxoOps = nil
+			_totalInput, _totalOutput, _fees = 0, 0, 0
+			_err = errors.Errorf("ConnectTransaction: Recovered from panic: %v", r)
+			safeUtxoView.primaryView = safeUtxoView.backupView.CopyUtxoView()
+		}
+	}()
 	// Connect the transaction to the primary view.
 	utxoOpsForTxn, totalInput, totalOutput, fees, err := safeUtxoView.primaryView.ConnectTransaction(
 		txn, txHash, blockHeight, blockTimestampNanoSecs, verifySignatures, ignoreUtxos,
@@ -51,6 +81,8 @@ func (safeUtxoView *SafeUtxoView) ConnectTransaction(
 		return nil, 0, 0, 0, errors.Wrapf(err, "TryConnectTransaction: Problem connecting txn on copy view")
 	}
 
+	revertBackupToPrimary = true
+
 	// Connect the transaction to the backup view.
 	_, _, _, _, err = safeUtxoView.backupView.ConnectTransaction(
 		txn, txHash, blockHeight, blockTimestampNanoSecs, verifySignatures, ignoreUtxos,

lib/block_view_utils_test.go

@@ -0,0 +1,75 @@
+package lib
+
+import (
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestSafeUtxoView(t *testing.T) {
+	setBalanceModelBlockHeights(t)
+	setPoSBlockHeights(t, 1, 3)
+	chain, params, db := NewLowDifficultyBlockchain(t)
+
+	mempool, miner := NewTestMiner(t, chain, params, true /*isSender*/)
+	_, err := miner.MineAndProcessSingleBlock(0 /*threadIndex*/, mempool)
+	require.NoError(t, err)
+	_, err = miner.MineAndProcessSingleBlock(0 /*threadIndex*/, mempool)
+	require.NoError(t, err)
+	// We take the block tip to be the blockchain height rather than the
+	// header chain height.
+	savedHeight := chain.blockTip().Height + 1
+	// We build the testMeta obj after mining blocks so that we save the correct block height.
+	testMeta := &TestMeta{
+		t:           t,
+		chain:       chain,
+		params:      params,
+		db:          db,
+		mempool:     mempool,
+		miner:       miner,
+		savedHeight: savedHeight,
+	}
+	_registerOrTransferWithTestMeta(testMeta, "", senderPkString, m0Pub, senderPrivString, 70)
+
+	safeUtxoView := NewSafeUtxoView(NewUtxoView(db, params, nil, nil, nil))
+
+	safeUtxoView.primaryView.PublicKeyToDeSoBalanceNanos = nil
+
+	// Construct a basic transfer
+	txn := &MsgDeSoTxn{
+		TxInputs:    []*DeSoInput{},
+		TxOutputs:   []*DeSoOutput{},
+		TxnFeeNanos: 50,
+		PublicKey:   nil,
+		TxnMeta:     &BasicTransferMetadata{},
+		TxnNonce:    &DeSoNonce{ExpirationBlockHeight: 10, PartialID: 1000},
+	}
+
+	// Sign the txn
+	txn.PublicKey = m0PkBytes
+	_signTxn(t, txn, m0Priv)
+	txnHash := txn.Hash()
+
+	_, _, _, _, err = safeUtxoView.ConnectTransaction(
+		txn,
+		txnHash,
+		3,
+		1e9,
+		false,
+		false,
+	)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Recovered from panic")
+
+	safeUtxoView.primaryView.PublicKeyToDeSoBalanceNanos = make(map[PublicKey]uint64)
+	safeUtxoView.backupView.PublicKeyToDeSoBalanceNanos = nil
+
+	_, _, _, _, err = safeUtxoView.ConnectTransaction(
+		txn,
+		txnHash,
+		3,
+		1e9,
+		false,
+		false,
+	)
+	require.NoError(t, err)
+}