wtxmgr: fix broadcast existing transaction

This commit ensures the wallet won't enter an inconsitent state
by checking tx confirmation before adding credit.
Without this fix, In the case the existing transaction is already
confirmed on-chain the flow updates the bucketUnminedCredits but
without adding en entry also to the bucketUnmined resulting in
inconsistent state.
This commit is contained in:
Roei Erez 2019-10-28 12:24:19 +02:00
parent ed825b0fd9
commit 1c43ed9294
2 changed files with 94 additions and 16 deletions

View file

@ -404,7 +404,9 @@ func (s *Store) addCredit(ns walletdb.ReadWriteBucket, rec *TxRecord, block *Blo
if existsRawUnminedCredit(ns, k) != nil {
return false, nil
}
if existsRawUnspent(ns, k) != nil {
if _, tv := latestTxRecord(ns, &rec.Hash); tv != nil {
log.Tracef("Ignoring credit for existing confirmed transaction %v",
rec.Hash.String())
return false, nil
}
v := valueUnminedCredit(btcutil.Amount(rec.MsgTx.TxOut[index].Value), change)

View file

@ -162,7 +162,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstRecvTx.MsgTx().TxOut[0].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstRecvTx.Hash(),
Index: 0,
}: {},
@ -189,7 +189,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstRecvTx.MsgTx().TxOut[0].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstRecvTx.Hash(),
Index: 0,
}: {},
@ -216,7 +216,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: btcutil.Amount(TstRecvTx.MsgTx().TxOut[0].Value),
unc: 0,
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstRecvTx.Hash(),
Index: 0,
}: {},
@ -241,7 +241,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: btcutil.Amount(TstRecvTx.MsgTx().TxOut[0].Value),
unc: 0,
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstRecvTx.Hash(),
Index: 0,
}: {},
@ -257,7 +257,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstRecvTx.MsgTx().TxOut[0].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstRecvTx.Hash(),
Index: 0,
}: {},
@ -284,7 +284,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: btcutil.Amount(TstDoubleSpendTx.MsgTx().TxOut[0].Value),
unc: 0,
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstDoubleSpendTx.Hash(),
Index: 0,
}: {},
@ -343,7 +343,7 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstSpendingTx.MsgTx().TxOut[0].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 0,
}: {},
@ -369,11 +369,11 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstSpendingTx.MsgTx().TxOut[0].Value + TstSpendingTx.MsgTx().TxOut[1].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 0,
}: {},
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 1,
}: {},
@ -395,11 +395,11 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: btcutil.Amount(TstSpendingTx.MsgTx().TxOut[0].Value + TstSpendingTx.MsgTx().TxOut[1].Value),
unc: 0,
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 0,
}: {},
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 1,
}: {},
@ -415,11 +415,11 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: btcutil.Amount(TstSpendingTx.MsgTx().TxOut[0].Value + TstSpendingTx.MsgTx().TxOut[1].Value),
unc: 0,
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 0,
}: {},
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 1,
}: {},
@ -435,11 +435,11 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) {
bal: 0,
unc: btcutil.Amount(TstSpendingTx.MsgTx().TxOut[0].Value + TstSpendingTx.MsgTx().TxOut[1].Value),
unspents: map[wire.OutPoint]struct{}{
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 0,
}: {},
wire.OutPoint{
{
Hash: *TstSpendingTx.Hash(),
Index: 1,
}: {},
@ -1522,6 +1522,82 @@ func TestInsertMempoolTxAlreadyConfirmed(t *testing.T) {
checkStore()
}
// TestInsertMempoolTxAfterSpentOutput ensures that transactions that were
// both confirmed and spent cannot be added as unconfirmed.
func TestInsertMempoolTxAfterSpentOutput(t *testing.T) {
t.Parallel()
store, db, teardown, err := testStore()
if err != nil {
t.Fatal(err)
}
defer teardown()
// First we add a confirmed transaction to the wallet.
b100 := BlockMeta{
Block: Block{Height: 100},
Time: time.Now(),
}
cb := newCoinBase(1e8)
cbRec, err := NewTxRecordFromMsgTx(cb, b100.Time)
if err != nil {
t.Fatal(err)
}
commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) {
if err := store.InsertTx(ns, cbRec, &b100); err != nil {
t.Fatal(err)
}
err := store.AddCredit(ns, cbRec, &b100, 0, false)
if err != nil {
t.Fatal(err)
}
})
// Then create a transaction that spends the previous tx output.
b101 := BlockMeta{
Block: Block{Height: 101},
Time: time.Now(),
}
amt := int64(1e7)
spend := spendOutput(&cbRec.Hash, 0, amt)
spendRec, err := NewTxRecordFromMsgTx(spend, time.Now())
if err != nil {
t.Fatal(err)
}
commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) {
// We add the spending tx to the wallet as confirmed.
err := store.InsertTx(ns, spendRec, &b101)
if err != nil {
t.Fatal(err)
}
err = store.AddCredit(ns, spendRec, &b101, 0, false)
if err != nil {
t.Fatal(err)
}
// We now adding the original transaction as mempool to simulate
// a real case where trying to broadcast a tx after it has been
// confirmed and spent.
if err := store.InsertTx(ns, cbRec, nil); err != nil {
t.Fatal(err)
}
err = store.AddCredit(ns, cbRec, nil, 0, false)
if err != nil {
t.Fatal(err)
}
})
// now we check that there no unminedCredit exists for the original tx.
commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) {
k := canonicalOutPoint(&cbRec.Hash, 0)
if existsRawUnminedCredit(ns, k) != nil {
t.Fatalf("expected output to not exist " +
"in unmined credit bucket")
}
})
}
// TestOutputsAfterRemoveDoubleSpend ensures that when we remove a transaction
// that double spends an existing output within the wallet, it doesn't remove
// any other spending transactions of the same output.