Missed transactions (because of risky parent?)

[GoogleCodeExporter]

I think I've seen this at least twice. First a transaction is considered risky 
because its parent is considered risky. This is because it has a 0.00001 output 
(probably because of the new fee rules).

But then when the block arrives, it is still not being included in the wallet. 
That's the main bug I'd say.

18:46:56.537 [NioClientManager] MemoryPool - [24.27.95.117]:8333: Peer 
announced new transaction [1] 
2e50349e190140e2a902af7d2afd5c6aca92cda5a79e343cb8438a2632d1a717
18:46:56.885 [NioClientManager] Peer - [24.27.95.117]:8333: Downloading 
dependencies of 2e50349e190140e2a902af7d2afd5c6aca92cda5a79e343cb8438a2632d1a717
18:46:56.893 [NioClientManager] Peer - [24.27.95.117]:8333: Downloaded 
dependency of 2e50349e190140e2a902af7d2afd5c6aca92cda5a79e343cb8438a2632d1a717: 
c64343b3ac7498e135dab49ff54fe411844f79f1447b4c7814f8b8aee8334714
18:46:57.172 [NioClientManager] Peer - [24.27.95.117]:8333: Bottomed out dep 
tree at 9f720bbc20449a2dceabc35c78bcafa227be83db5b30fa1e32aa8d0f6002f006
18:46:57.177 [NioClientManager] Peer - [24.27.95.117]:8333: Dependency download 
complete!
18:46:57.186 [NioClientManager] Wallet - Pending transaction 
2e50349e190140e2a902af7d2afd5c6aca92cda5a79e343cb8438a2632d1a717 was considered 
risky: Risky due to non-standard tx 
c64343b3ac7498e135dab49ff54fe411844f79f1447b4c7814f8b8aee8334714
18:52:07.249 [NioClientManager] AbstractBlockChain - 2 blocks per second
18:52:07.257 [NioClientManager] Wallet - TX 
2e50349e190140e2a902af7d2afd5c6aca92cda5a79e343cb8438a2632d1a717 not found 
despite being sent to wallet

See full log in the attachment. Bitcoinj 0.11.1. Wallet address: 
11Zcrc76CnVDezQCLihtzz5RAupcSFS5n

Original issue reported on code.google.com by andreas....@gmail.com on 17 Apr 2014 at 4:05

Attachments:

• wallet.2014-04-16.-944159928.log.gz

[GoogleCodeExporter]

Ah, yes. The wallet should store risky transactions. We could change it so 
there's a new pool for that, or accept them into the pending pool as normal and 
just not select them (but they would still appear in the tx list and trigger 
event listeners). I guess the question is, how much activity should take place 
if a risky tx appears. Like if there's a onReceived callback and the balance 
doesn't change even for ESTIMATED, then would that be confusing.

Original comment by mh.in.en...@gmail.com on 17 Apr 2014 at 5:18

[GoogleCodeExporter]

I don't understand why should it store risky transactions? And why do those 
transactions not show up when they are confirmed by the blockchain? (They 
should -- in this case they are not risky any more)

Original comment by andreas....@gmail.com on 17 Apr 2014 at 10:32

[GoogleCodeExporter]

Because of Bloom filtering. The remote peer sent us the tx data previously so 
assumes we still have it. It won't send it again when a block turns up with it, 
if we were previously connected for broadcast. So we have to keep them around 
in case this happens. It's a design flaw in the risk analysis feature but one 
nobody noticed. A workaround is to disable risk analysis until there's a fix.

Original comment by mh.in.en...@gmail.com on 18 Apr 2014 at 8:40

[GoogleCodeExporter]

That's bad news. I still get the spam transactions forwarded, so I assume the 
majority of other users also get it.

Maybe we can just disable risk analysis based on "parents" for now? I think we 
wanted to this anyway because of the stack overflow problems.

Original comment by andreas....@gmail.com on 18 Apr 2014 at 8:43

[GoogleCodeExporter]

Still I wonder if nodes don't forward transactions with the block if you 
already got them how can an SPV wallet know it just got confirmed? Somehow you 
need to know which (relevant) transactions are part of the block, right?

As for your questions in #1, I'd say risky transactions should not show up in 
user visible lists, because that's the whole point of risk analysis. Also, we 
wanted to get rid of spam. Maybe in future we could have multiple levels of 
"riskyness", but for now I think we should aim at transactions that should not 
be visible to users unless they are confirmed by the blockchain.

I don't care much about the invocation of callbacks as long as the framework 
doesn't suddenly start to "hammer". Apps can always check if transactions they 
got notified about are relevant/risky/whatever... 

Original comment by andreas....@gmail.com on 18 Apr 2014 at 9:01

[GoogleCodeExporter]

BIP 70 explains how it works. The block is sent with (simplifying) a list of tx 
hashes.

Yes we can disable dependency download but it doesn't fix the real problem 
which is that risky transactions that confirm anyway will be missed.

Original comment by mh.in.en...@gmail.com on 18 Apr 2014 at 9:04

[GoogleCodeExporter]

I'm not sure if keeping risky transactions in the wallet -- separate pool or 
not -- is the right way to do. After all, one reason for risk analysis is to 
prevent DOS due to spam using up resources like heap. How would we clean up the 
risky pool? Etc.

Can we instead just re-request transactions which we don't know?

Original comment by andreas....@gmail.com on 18 Apr 2014 at 6:29

[GoogleCodeExporter]

We don't try to be DoS resistant today anyway so I'm not worried about that. 
It'd be easier to just make it remember all relevant transactions rather than 
have complicated refetch logic.

Original comment by mh.in.en...@gmail.com on 19 Apr 2014 at 7:55

[GoogleCodeExporter]

Coming back to the question about callbacks again. I think I misunderstood the 
question when I first read it.

onReceive should only be called if a transaction is allowed into pending or 
confirmed pools. If it only goes into risky it should imho be treated as if it 
was never received, means no callbacks called. Likewise the risky pool should 
not count towards any of the balances. In future we might add a "risky balance" 
but currently I don't see much use for that.

Maybe we should work towards a P2P protocol change that always sends all 
filter-matching transactions with blocks, regardless of what happened before. 
It's only a small overhead in traffic but I think it can make a big difference 
in reliability. After all, if a node sends a pending transaction how can it be 
sure that it has been received by the remote peer? Currently it just guesses 
and includes the transaction with the block based on that guess, right?

Original comment by andreas....@gmail.com on 22 Apr 2014 at 11:21

[GoogleCodeExporter]

A P2P protocol change is really not worth it. The current protocol is fine. All 
we have to do is not throw away data we're sent - hardly a big deal. The only 
question is where to put it and how to treat it.

Not invoking callbacks is fine by me. That said I'm not keen on introducing yet 
another pool (really - map of states). It might make more sense to just assume 
risk analysis is fast and check when needed.

Original comment by mh.in.en...@gmail.com on 22 Apr 2014 at 12:00

[GoogleCodeExporter]

Resolved by 

https://github.com/bitcoinj/bitcoinj/commit/467124a2b395030c6b29dcceeefaa4a81467
489d

Original comment by mh.in.en...@gmail.com on 21 May 2014 at 5:27

• Changed state: Fixed