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

db: txindex known without blockheight #3265

Closed
m-schmoock opened this issue Nov 15, 2019 · 4 comments
Closed

db: txindex known without blockheight #3265

m-schmoock opened this issue Nov 15, 2019 · 4 comments
Milestone

Comments

@m-schmoock
Copy link
Collaborator

m-schmoock commented Nov 15, 2019

Issue and Steps to Reproduce

An data consistency assert added by commit ed23875 (merged yesterday into master) causes problems on my testnet and mainnet nodes. The added assert checks that if a transaction index is known a blockheight must also be known or wise versa.

If your node is affected it can be reproduced by:

lightning-cli listtransactions

If the above command asserts, its very likely your node has corrupt data in sqlite db.

SQL query to verify

> sqlite3 ~/.lightning/lightningd.sqlite3

SELECT  HEX(t.id), t.blockheight, t.txindex FROM transactions t WHERE t.txindex > 0 and t.blockheight IS NULL;

(for my mainnet node:)
CB785ABCC3971F94607A270009D161460D1D63C8E915CFEC933320DF37ED372F||1663
5E35A558D3061E6EC6D91A89346639DEDAAA704281A4F7B597D63EB6591307B8||855

Notes

This cannot be 'fixed' by using dev-rescan-outputs. We could add a dev-rescan-transactions or add functionality to dev-rescan-outputs and call it on startup if we detect this inconsistency in the database.

@m-schmoock
Copy link
Collaborator Author

It may very well be that current code does not reproduce this, as the code that lead to it has already been fixed, but the databases with corruptions are still out in the wild...

@cdecker
Copy link
Member

cdecker commented Nov 15, 2019

I see, the assertion might be a bit overzealous. The blockheight might be under on a reorg, leaving the txindex set. We should just remove the assertion and add the index only if we also have a block height.

@cdecker cdecker added this to the 0.7.4 milestone Nov 15, 2019
@m-schmoock
Copy link
Collaborator Author

I also thought about reorgs on code that didnt handle DB update correctly. But I doubt this was the reason since I have 4 such TX on two nodes. Too many to be a coincidence.

In any case, the txindex should be erased from the DB if the blockheight is unknown for whatever reason. If we check this we could keep the assert...

It would also be good to know what causes this...

@cdecker
Copy link
Member

cdecker commented Nov 16, 2019

We'll likely not unset the txindex on reorg at least since we rely heavily on the ON DELETE foreign keys in the DB which cannot really be applied to non-foreign keys. I'd prefer to gate the txindex being shown based on whether we have a blockheight.

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

No branches or pull requests

2 participants