-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
Inconsistency in listsinceblock
between lastblock
and confirmations
#19338
Comments
Indeed that appears to be a race condition. To fix this either the function needs to detect the tip changing or it needs to hold cs_main. |
It seems that `listsinceblock` is not atomic and could provide results that are inconsistent with each other. The workaround checks if the chain tip moved while reading `listsinceblock` and retries if it did. This is similar to how atomicity is achieved for `listunspent` (in `Query::list_unspent_raw`) and previously for `listtransactions` (before being replaced by `listsinceblock`). The bug was reported here: bitcoin/bitcoin#19338
@shesek what version of bitcoin are you seeing this with? I think this issue should have been fixed by f7ba881 from #17954 (merged a few days after the 0.20 branch). Trying your reproduction script (nice script!) on current master 8ef15e8, I saw confirmations increase in lockstep with lastblock: {"lastblock":"01a1db130aae8fe7c878dffa6cee4f6251b0a28bac42383a014e1fd0c845bdd9","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",3,102]}
{"lastblock":"57d422331f2d7f8dd66e5e52a2625f729368360bcee66f7ab647d466c0f3282c","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",4,102]}
{"lastblock":"57d422331f2d7f8dd66e5e52a2625f729368360bcee66f7ab647d466c0f3282c","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",4,102]}
{"lastblock":"47d5ea88afb4672c45c4087ed6cd1f13c6640c6a346bcbb1e2ba7f9e3aa24372","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",5,102]}
{"lastblock":"7bbb7c7141faced32c82f5760343a23a04ea7929e69cd3fefee41720d14fc69c","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",6,102]}
{"lastblock":"0f66cb1ecc1bed21b250a9bbb67623d9e7624fde44cdc12e947c48a40548f9b8","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",7,102]}
{"lastblock":"464fd253141187ca5293e633c61582c747a333bc5c56300c9c54f256462c896a","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",9,102]}
{"lastblock":"5d278017fad7d146c0fddc0600d70e112743524d3599c480366bcc89c75d4bab","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",10,102]}
{"lastblock":"6b69ec48239f2c4a9cec13f284501473329777825251ffaa73c7284f657a6d97","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",11,102]}
{"lastblock":"3e4c3929c89d7f2309d1e98636c252c8cdbbe4d0669dc8203042d4c24b679b7e","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",12,102]}
{"lastblock":"3e4c3929c89d7f2309d1e98636c252c8cdbbe4d0669dc8203042d4c24b679b7e","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",12,102]}
{"lastblock":"5b34d33d545e114301746236feda3abf5e7066c84b612e19d43f700aca724510","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",14,102]}
{"lastblock":"5b34d33d545e114301746236feda3abf5e7066c84b612e19d43f700aca724510","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",14,102]}
{"lastblock":"4a5750028b953ec523420b02bb5f9145c78acfb98ae816efb8bed8b0e8ac5d9c","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",17,102]}
{"lastblock":"4a5750028b953ec523420b02bb5f9145c78acfb98ae816efb8bed8b0e8ac5d9c","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",17,102]}
{"lastblock":"62ac4ab1fc763d8b583d6818568c96b907e9133119bda394f2f019536859d661","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",19,102]}
{"lastblock":"62ac4ab1fc763d8b583d6818568c96b907e9133119bda394f2f019536859d661","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",19,102]}
{"lastblock":"37b190f76332537692212da0302f4f78d4d8997983a35f8a906addc0a28977dc","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",20,102]}
{"lastblock":"37b190f76332537692212da0302f4f78d4d8997983a35f8a906addc0a28977dc","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",20,102]}
{"lastblock":"37b190f76332537692212da0302f4f78d4d8997983a35f8a906addc0a28977dc","tx":["a1ae1cc64e6b8f5e012af93ab227d41236d3ad671f20f50e53230936a66afe4d",20,102]}
re: #19338 (comment)
I don't think these things are true. Wallet code shouldn't need to be concerned with node tip to report internally consistent results, and it shouldn't hold cs_main |
Retested master 8ef15e8 with 100 blocks and still didn't see the bug (didn't check exhaustively but piped jq output to Also tested v0.20.0 tag a62f0ed and did see the bug (same lastblock hash repeated with different confirmation numbers). Also confirmed bugfix commit f7ba881 from #17954 did not exhibit the bug and while parent commit bc96a9b did have the bug. All this suggests f7ba881 from #17954 does fix this bug, which I'd expect, because this is exactly the type of inconsistency #17954 was intended to fix. @shesek, it's be great if you could verify the fix not just to provide confirmation, but also to ensure the fix doesn't have it's own problems since Another thing that's unclear is how important the bug is and whether it'd be useful to backport a fix. I'd think the behavior described was probably longstanding and not a regression. If a backport should be done, it might be worth only backporting commit f7ba881 and the new interfaces/chain methods it depends and not backporting all of #17954. That way the backport is strictly limited to the |
I verified that this is no longer an issue against current master (e3fa3c7).
It wouldn't be ideal if issuing I used this modified script to check both for consistent
At least for my specific case, I worked around the bug and don't mind it not being backported. But others using this might disagree... |
Thanks for checking and verifying that slightly more stale information returned by the fix doesn't cause problems for the external index. Also thanks for the new test script. Since you have a workaround, and since this bug has probably been around a long time, and since backporting the fix requires breaking up #17954 or pulling in unrelated changes, I think I'd opt not to backport f7ba881 from #17954 right now. But we could reconsider if more problems are reported. |
Agreed, I think so too. Thanks for helping diagnose this! |
While still using it when an older Bitcoin Core release is detected. See: bitcoin/bitcoin#19338
While still using it when an older Bitcoin Core release is detected. See: bitcoin/bitcoin#19338
While still using it when an older Bitcoin Core release is detected. See: bitcoin/bitcoin#19338
It seems like issuing
listsinceblock
rpc calls while bitcoind is processing blocks can result inlastblock
andconfirmations
values that disagree with each-other -- the number of confirmations is not relative to thelastblock
.I'm able to reliably reproduce this by issuing
listsinceblock
calls from ablocknotify
handler. Here's a reproduction script:This is the output of the
jq
command on the last line of the script, with Bitcoin Core 0.20.0:Notice how
lastblock
is reported as4c2ccc8d..
with 13 confirmations for tx58cf095f...
, then later with 14 confirmations for the same block and tx, then later with 15.You may need to generate more than 20 blocks if you're not able to catch this. For the tests I ran, it seems like 20 (or even 10) was sufficient.
The text was updated successfully, but these errors were encountered: