search

janoside / btc-rpc-explorer

Database-free, self-hosted Bitcoin explorer, via RPC to Bitcoin Core.
https://bitcoinexplorer.org
MIT License
1.48k stars 1.11k forks source link

fix confirmed block count for unconfirmed txns #606

Closed ExperiBass closed 6 months ago

ExperiBass commented 6 months ago

the check for unconfirmed transactions only checks if the txBlockHeight variable exists; since unconfirmed txns have a height of -1, it "exists", and the check passes. this pr checks txBlockHeight > -1.
before after

...dunno why the images wont load...

janoside commented 6 months ago

@ExperiBass Interesting. I wanted to confirm the situation before merging, but I'm unable to reproduce this issue. To test, I'm browsing to /mempool-transactions then clicking on an address involved in any of those unconfirmed transactions, and using master-branch code (without your change) on the address-details page I correctly see the unconfirmed TX as having zero confirmations.

Can you give me any more details about your setup?

ExperiBass commented 6 months ago

squint im also unable to reproduce it now... im also on master, 2e3797e.

I was lookin at the address bc1ptykavwhg6cl8e2v976f8swpw64nzdt7ec9vp749ja0ha74gc4ajsuvgu8x, im gonna blame js for this one :/

ExperiBass commented 6 months ago

i say that and immediately find bc1pl7y3qjat2kljkjj0g8mh27k68206m3yfrmhxk8wnyzlhaemdfngqly0v36 lmao image image

janoside commented 6 months ago

Interesting. I still haven't found one yet, but I'm still trying. What address API implementation are you using? I assume electrum-based, but what backend electrum server (type and version) is configured?

ExperiBass commented 6 months ago

im using electrs 0.10.0, nothin special configured other than a larger max return value. I wonder if its only CPFPed transactions? Lookin at bc1pw6yynj65ty4peksu8ehe8g0j5jurqznvyez7yfxtly8lux9yj3zqjpn6uk, and all the CPFPed transactions it has in the mempool all display the bug.

janoside commented 6 months ago

Ah, I found one, and I also see the issue on the TX you referenced. Thanks for the fix and thanks for helping me reproduce. I'm going to test and likely merge.

janoside commented 6 months ago

Merged with a couple of tweaks.