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

fix confirmed block count for unconfirmed txns #606

Merged
merged 1 commit into from
Jan 1, 2024

Conversation

ExperiBass
Copy link
Contributor

@ExperiBass ExperiBass commented Dec 28, 2023

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
Copy link
Owner

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

ExperiBass commented Dec 29, 2023

i say that and immediately find bc1pl7y3qjat2kljkjj0g8mh27k68206m3yfrmhxk8wnyzlhaemdfngqly0v36 lmao
image
image

@janoside
Copy link
Owner

janoside commented Jan 1, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

janoside commented Jan 1, 2024

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 janoside merged commit c561ba6 into janoside:master Jan 1, 2024
2 checks passed
@janoside
Copy link
Owner

janoside commented Jan 1, 2024

Merged with a couple of tweaks.

@ExperiBass ExperiBass deleted the fix-unconfirmed-txns branch January 1, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants