Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

listsinceblock incorrectly showing some conflicted transactions #10656

Open
RHavar opened this issue Jun 23, 2017 · 9 comments
Open

listsinceblock incorrectly showing some conflicted transactions #10656

RHavar opened this issue Jun 23, 2017 · 9 comments

Comments

@RHavar
Copy link
Contributor

@RHavar RHavar commented Jun 23, 2017

Normally when a transaction is conflicted, listsinceblock will show it as having negative confirmations, representing how long ago it conflicted. However, looking at the results of my listsinceblock shows dozens of conflicted transactions with 0 confirmations (as well as transactions whos parents have long been forgotten by the network).

An example:

{
  "account": "",
  "address": "1AM7qmzyzkZecxn5hTPUCbBbog3r3YbVyE",
  "category": "receive",
  "amount": 0.00684870,
  "label": "",
  "vout": 0,
  "confirmations": 0,
  "trusted": false,
  "txid": "f108f58235e35f3eb07d24d9d330f6098234137dd505bbd19ff249d809254f2d",
  "walletconflicts": [
  ],
  "time": 1493696422,
  "timereceived": 1493696422,
  "bip125-replaceable": "unknown"
}

Which would make you think it's a pending receive, so lets dive deeper:

bitcoin-cli gettransaction f108f58235e35f3eb07d24d9d330f6098234137dd505bbd19ff249d809254f2d
{
  ....
  "hex": "0100000001975c0acaefcc4abc471c5d9fe8de6ee5d6e76e7f01a089466f0a73c5f23dc36c000000006b48304502210088baffcff66ade8770a802888c868071291be4f6c6b7427e9938dbf2c9df6ee6022028a9a1af3e7e0ef4cdf42b3b6cc3113787e4db29922c1b90e182b2f1665be83e012102efdaa4cbd4fb4edd41978403a4343786462fefa514f98cbe1894f7053a1216c2ffffffff0146730a00000000001976a9146687296e39b87666fcbff34738c984eca686451e88ac00000000"
}

Decoding that, you'll see it's sourcing:
{ "txid": "6cc33df2c5730a6f4689a0017f6ee7d6e56edee89f5d1c47bc4accefca0a5c97", "vout": 0 }

Which is a transaction that has not confirmed, nor in my nodes mempool. Fortunately we can find it on tradeblock, and see it's conflicted with dd0ab2618d4df044d369b56ce96f6f8d9dca6e04f4c180fa73e8ace20b3653e3 which has long confirmed.

So this means that our original transaction is just sitting in listsinceblock is actually conflicted and impossible to ever confirm, yet not marked as such.

@promag

This comment has been minimized.

Copy link
Member

@promag promag commented Dec 7, 2017

IMO f108f58235e35f3eb07d24d9d330f6098234137dd505bbd19ff249d809254f2d is not conflicted, it's source is.

What would you like to have in the response?

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Dec 8, 2017

This would require a pretty decent refactor of the wallet as we'd have to store dependants of wallet transactions in the wallet, which would also be a pretty big blowup of wallet on-disk size.

@RHavar

This comment has been minimized.

Copy link
Contributor Author

@RHavar RHavar commented Dec 8, 2017

What would you like to have in the response?

Well the whole point of listsinceblock is to return transactions you should care about (for processing, or what ever). So in this case the API response is identical to "you have a pending receive" but the reality is the transaction is orphaned and it's now impossible for it to ever confirm. This is a distinction worth making.

(FWIW, I don't care too much about this issue as I don't use listsinceblock as despite having the perfect API for me -- it's pretty much unusuable due to bugs/quirks/bad-behavior/etc. This issue was an attempt at documenting some of the problems I found, but I think it might just be better to deprecate the whole command)

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Dec 8, 2017

@RHavar

This comment has been minimized.

Copy link
Contributor Author

@RHavar RHavar commented Dec 8, 2017

but anything else would be a huge blow-up in wallet size just to provide a non-promise that we could tell a user when something has a conflicted dependant (as the dependant could be 100 deep and we can't store that much).

Really? I don't have the Qt stuff installed, so I can't really see -- but what happens in the UI? Surely you don't see an impossible-to-confirm transaction as perpetually "pending"... That'd be a pretty serious bug.

I'm curious what other issues you have with listsinceblock? I assume there's issues for them?

Can't really remember how many of them have issues or not, but I can file them if there's any interest in making it work properly.

But really the first step in fixing it though would be writing some extensive description of what the behavior should be (especially in the edge cases: conflicts, replaced, this issue, sending-to-self etc) and then it would be a lot easier to see if it makes sense or not -- and report deviations of it. Right now it's pretty wtf, and you can't tell if it's intentional or not. Like the blockhash filter doesn't even work for conflicted transactions (at least when I tried a year ago)

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Dec 8, 2017

Indeed, also in Qt, if you have a transaction who's dependant four-parents-deep is conflicted the transaction will not neccessarily be marked conflicted, as its not possible to know that without storing an immense amount of data. The only thing we can determine is that the transaction failed to get into our mempool. Of course if the transaction itself is conflicted we track that, but as we don't store the parents of all wallet transactions (recursively) we cant figure out that it is dependant-conflicted.

As for returning conflicted transactionsr repeatedly, I opened #11853.

@RHavar

This comment has been minimized.

Copy link
Contributor Author

@RHavar RHavar commented Dec 8, 2017

as its not possible to know that without storing an immense amount of data.

You only really need to store unconfirmed ancestors, which is currently bounded at pretty low number. I think the only issue I see is just the amount of fragile and complex you need to do in order to handle things like reorgs :/

In my own wallet, I just mark any transaction that gets evicted from the mempool as conflicted. Although, I suppose you can argue that it's really orphaned and not conflicted, but I'm not sure that is a distinction particularly useful to anyone. But if it is, you could always just have an "is_orphaned" flag or something

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Dec 8, 2017

You only really need to store unconfirmed ancestors

That is an option, but in the case of even a 1-block reorg you'd be back in the same state where you can't guarantee anything about reporting conflicts. Also, note that even storing unconfirmed ancestors would potentially be a huge blowup in wallet size for some users, as it could potentially include up to 25 transactions of up to 100KB per wallet transaction.

In my own wallet, I just mark any transaction that gets evicted from the mempool as conflicted

Indeed, my point above was that a transaction which is not in mempool is actually a pretty decent proxy for "quite possibly conflicted, but even if its not, it not being in your mempool probably means its not going to be confirmed any time soon, so just treat it was conflicted".

@RHavar

This comment has been minimized.

Copy link
Contributor Author

@RHavar RHavar commented Dec 8, 2017

Agree about not being in the mempool is a good enough proxy. But I don't think your solution is ideal because you need more than "is_not_in_mempool" but also a time/block or something.

Because I care if the transaction was orphaned recently (for instance, I probably want to take action based on this fact) but I don't really want to see for eternity (when I'm only asking for recent events)

@MarcoFalke MarcoFalke added the Wallet label Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.