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

Create txdb credit for known key paths when confirming #262

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

pinheadmz
Copy link
Member

This is a port of bcoin-org/bcoin#829

Refer to original post for summary and discussion.

Note the hsd wallet has one extra detail in the txdb.confirm() function, it adds coins from the tx input to a view, which gets passed to connectNames() -- clearly not necessary in bcoin.

Since this PR only affects the handling of tx outputs, we should be ok. But reviewers more familiar with hsd might wanna double check me on that.

The reason we don't need to do anything for the inputs was discussed offline:

When txdb confirms a tx, it assumes that all input coins are confirmed. In fact, this is consensus for obvious reasons. If it sees an input it doesn't own, it ignores it. Even if the tx gets unconfirmed and re-confirmed, txdb will not recognize that credit or create a new credit. That makes sense because if we want to get that output/credit/coin into the txdb - we must go back and txdb.confirm() the original tx that created it, and I believe that is the case the PR already addresses. We wouldn't really be able to create a new coin at this point anyway, since all we know if the outpoint (hash/index), not the script or sender address or value...

@@ -18,7 +18,7 @@ const KeyRing = require('../lib/primitives/keyring');
const Input = require('../lib/primitives/input');
const Outpoint = require('../lib/primitives/outpoint');
const Script = require('../lib/script/script');
const PrivateKey = require('../lib/hd/private.js');
const HDPrivateKey = require('../lib/hd/private');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol - this .js was leftover from an earlier port #97 I guess I opened the PR on hsd before it was finished on bcoin!

@tynes
Copy link
Contributor

tynes commented Jan 23, 2020

Hey @pinheadmz, looks like this needs a rebase

@pinheadmz
Copy link
Member Author

@tynes thanks, pushed.

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.

4 participants