Nodes forwarding transactions with empty vins

TheBlueMatt commented at 2:07 am on November 1, 2013: member

debug.log shows this is definitely appearing more than the number of modified nodes (and we’re setting DoS for nodes doing this). Theory is wallets getting transactions cleared and stored with 0-bytes instead of their value.

ghost commented at 2:20 am on November 30, 2013: none

I am seeing this in my log for the latest master too. The nodes eventually get banned for it.The nodes sending empty transactions are running 0.8.5 (I’m only seeing 0.8.5 nodes, and 0.8.99). Of my peers I’m seeing at least one vin empty ban score increase every 10 minutes.

sipa commented at 10:37 am on November 30, 2013: member

My assumption here (has been for a while, I haven’t had time to dig in) that at some point a mapWallet[hash] or wallettx.vtxPrev[hash] is accessed with an inexisting transaction hash, resulting in empty transaction that get relayed.

ghost commented at 11:06 am on November 30, 2013: none

https://blockchain.info/rejected

Curious that they’re seeing huge blocks of them all at the exact same second. Wouldn’t it be random if it was part of the wallet transaction resending function? Almost every instance of the error in their public logs is big blocks all occurring within a second of each other.

laanwj commented at 2:47 pm on November 30, 2013: member

That’d make sense. These are all the places where mapWallet[…] is used:

rpcwallet.cpp: const CWalletTx& wtx = pwalletMain->mapWallet[hash]; → count is checked beforehand
wallet.cpp: mapWalletPrev[txWalletPrev.GetHash()] = &txWalletPrev;
wallet.cpp: tx = *mapWalletPrev[hash]; → count is checked beforehand
wallet.cpp: CWalletTx &coin = mapWallet[txin.prevout.hash]; → count is NOT checked beforehand
wallet.cpp: if(!ExtractDestination(mapWallet[txin.prevout.hash].vout[txin.prevout.n].scriptPubKey, address)) → count is NOT checked beforehand
walletdb.cpp: CWalletTx& wtx = pwallet->mapWallet[hash]; → record is populated from BDB afterwards
walletdb.cpp: WriteTx(hash, pwallet->mapWallet[hash]); → only called if hash was already in wallet

laanwj commented at 11:33 am on December 4, 2013: member

Another hypothesis (from @wtogami) is that this has to do with transactions in the wallet that are double spent (or otherwise linger around in the wallet a long time without being mined), and somehow end up being broadcasted empty instead of being ignored.

wtogami commented at 11:49 am on December 4, 2013: contributor

I have an old Litecoin testnet wallet.dat with stuck tx from 6 months ago from a time when I was testing fee violating tx creation. Today this wallet.dat causes rebroadcast of the old tx with empty vin and +10 banscore seen in the peer’s debug.log. I gave a copy of this wallet.dat to @laanwj for analysis.

laanwj commented at 1:11 pm on December 4, 2013: member

Some first conclusions from analysing @wtogami ’s wallet using pywallet:

There is no actual transaction in the wallet with 0 vins
The transaction id that is reported on the other side causing the misbehavior ban does not actually exist in the wallet:

2013-12-04 11:41:15 d21633ba23f70118185227be58a63527675641ad37967e2aa461559f577aec43 from /Satoshi:0.8.5.2/ was not accepted into the memory pool

This would make me think that either something goes wrong while broadcasting the transaction (which causes an empty transaction to be sent instead) or something goes wrong on the receiving end. In any case, the wallet itself does not seem to be corrupted.

I’ve added some debug information to CWalletTx::RelayWalletTransaction https://github.com/litecoin-project/litecoin/blob/master-0.8/src/wallet.cpp#L846; the following transactions are retransmitted by the wallet

02013-12-04 13:29:12 ResendWalletTransactions(1)
12013-12-04 13:29:12   Relaying wtx prev 6521bde62cbe8eaf6a4fd9db02116cf592dd4fe65c598dadbde78c0118432769
22013-12-04 13:29:12   Relaying wtx prev 6521bde62cbe8eaf6a4fd9db02116cf592dd4fe65c598dadbde78c0118432769
32013-12-04 13:29:12   Relaying wtx prev 729bd6dfd9981591089fbf96c5153964bb999414ce9fac4ac45038fcc364f2fd
4...
5(in RelayWalletTransaction(1ce01b9bf5776646fbff0a3d0fd0905379490e5b7b296fead5ca5e775b14f7e6) )
62013-12-04 13:29:12   Relaying wtx prev d21633ba23f70118185227be58a63527675641ad37967e2aa461559f577aec43
72013-12-04 13:29:12   Relaying wtx prev d21633ba23f70118185227be58a63527675641ad37967e2aa461559f577aec43
82013-12-04 13:29:12   Relaying wtx prev be0badd502f113001d0e05ab4f2d1790f1461a749cd88511716744c82537cf41
92013-12-04 13:29:12   Relaying wtx prev c873a065f74f37706b6196e74f5d085d55f06d29c98000fb99327f3fe3c11d8b

“wtx prev” are dependency transactions, transmitted in the first part of the function, “wtx” are the transactions themselves.

Whoah… our culprit, d21633ba23f70118185227be58a63527675641ad37967e2aa461559f577aec43 is between the “wtx prev” transactions.

It is being transmitted for a transaction that isn’t even transmitted itself: tx id 1ce01b9bf5776646fbff0a3d0fd0905379490e5b7b296fead5ca5e775b14f7e6.

Gettransaction for this transaction shows:

 0 {
 1  "amount" : -0.01900000,
 2  "fee" : -0.00100000,
 3  "confirmations" : 383,
 4  "blockhash" : "b7efacff198713bdc84db0d4cd0a750727d7a520aca4b4eb147cbb925c9d68f2",
 5  "blockindex" : 1,
 6  "blocktime" : 1385991737,
 7  "txid" : "1ce01b9bf5776646fbff0a3d0fd0905379490e5b7b296fead5ca5e775b14f7e6",
 8  "time" : 1386036379,
 9  "timereceived" : 1386036379,
10  "details" : [
11      {
12          "account" : "",
13          "address" : "mnwLq9mMv6tmARNmN6fQhDgjrofSQVkFru",
14          "category" : "send",
15          "amount" : -0.01900000,
16          "fee" : -0.00100000
17      }
18  ]

So it’s an old, confirmed transaction…

laanwj commented at 2:28 pm on December 4, 2013: member

Hah… found it

https://github.com/litecoin-project/litecoin/blob/master-0.8/src/wallet.cpp#L745 this if statement needs an else continue; currenly, if none of the two cases is hit, it stores an emptty transaction.

This would prevent invalid vtxPrevs from getting into the wallet, not existing ones getting broadcasted…

laanwj referenced this in commit d3ef9b00ec on Dec 4, 2013

wtogami referenced this in commit 8f16782290 on Dec 5, 2013

laanwj commented at 7:18 am on December 5, 2013: member

Should be solved by #3356 and #3357

laanwj closed this on Dec 5, 2013

laanwj referenced this in commit 1eb11e32e8 on Dec 5, 2013

laudney referenced this in commit 4dcd10dbe7 on Mar 19, 2014

kazcw referenced this in commit 126b476e3f on Jul 16, 2014

kazcw referenced this in commit ba80f517bb on Jul 16, 2014

kazcw referenced this in commit afa26719bf on Jul 16, 2014

kazcw referenced this in commit b0511fcf8f on Jul 17, 2014

kazcw referenced this in commit edde2dda82 on Jul 20, 2014

Gnos1s referenced this in commit c93e385af8 on Oct 29, 2014

destenson referenced this in commit 75ba0169ad on Jun 26, 2016

MarcoFalke locked this on Sep 8, 2021

Nodes forwarding transactions with empty vins #3190