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

Cleanup unconfirmed coinbase outputs from wallet #275

Closed
ignopeverell opened this issue Nov 16, 2017 · 7 comments
Closed

Cleanup unconfirmed coinbase outputs from wallet #275

ignopeverell opened this issue Nov 16, 2017 · 7 comments
Milestone

Comments

@ignopeverell
Copy link
Contributor

After some time mining with other nodes, we end up with some unconfirmed coinbase outputs in wallet.dat that linger there forever. They're very likely from orphaned blocks coinbase outputs. I think we could clean those up after a safe number of blocks.

Generally, we may also want to clean up unconfirmed outputs that have been stayed so for a while. Maybe after a few hundred blocks for coinbase outputs (only scenario here is orphaned blocks) and a few thousands for regular outputs (bad relay, blocks full, too low fee, etc).

@antiochp
Copy link
Member

Ah that's where they are coming from - you see them for coinbase outputs if you restart the mining node (the old one sticks around in Unconfirmed).
It hadn't clicked that we would see orphan blocks with only a couple of miners running.

For regular txs that end up never being confirmed (for whatever reason) - am I right in thinking these keys are at risk of "replay" if they ever get reused? The tx may not have been confirmed but the recipient may have seen the input.
So we'd want to clean them up by flagging them in some particular state to ensure they never get used in the future.

@ignopeverell
Copy link
Contributor Author

That sounds correct. Maybe we could just remove them and have the wallet keep some index of the latest keys (so always strictly increment)?

@antiochp
Copy link
Member

Yeah - I think keeping an incrementing counter/index in wallet.dat makes a lot of sense.

@sesam
Copy link
Contributor

sesam commented Mar 3, 2018

Current key_derivation_cache might need to go through big amounts of keys. If this happens also outside of wallet recovery, how about storing both key_id and iteration value i in wallet.dat?

And then the highest i in wallet.dat is the counter wanted above, right?

@ignopeverell ignopeverell modified the milestones: Testnet2, Beta Mar 26, 2018
@yeastplume
Copy link
Member

@ignopeverell I think you've addressed this in recent wallet work?

@ignopeverell ignopeverell modified the milestones: Beta / testnet3, Mainnet Jul 11, 2018
@ignopeverell
Copy link
Contributor Author

Sorry missed the question but I believe I did, where part of the output update process removes older unconfirmed outputs (I think a day old).

@hashmap
Copy link
Contributor

hashmap commented Jan 3, 2019

Should we close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants