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

Track coinbase transactions #64

Merged
merged 1 commit into from Nov 6, 2018

Conversation

Projects
None yet
2 participants
@andrewtoth
Copy link
Contributor

commented Nov 5, 2018

The current version of the server fails to account for coinbase transactions.

@andrewtoth andrewtoth force-pushed the andrewtoth:track-generated branch from 3921f9e to a77eeb4 Nov 5, 2018

@chris-belcher

This comment has been minimized.

Copy link
Owner

commented Nov 5, 2018

Thanks for this PR.

I've played around with regtest mode RPC to better understand these transactions, since I've never received newly-mined coins myself. I now understand that coinbase transactions are in the category "immature" and once they reach 100 confirmations they become "generate". Also coinbase transactions have the key "coinbase" in their inputs as a result of decoderawtransactions, because coinbase transactions don't have a previous input.

I think this line in check_for_new_txes should also check for ("generate", "immature")

if tx["category"] not in ("receive", "send"):
That would get triggered if a transaction arrives while the server is running, while the code you edited only gets triggered at the startup of the server.

Have you tested this with Electrum wallet? Does Electrum display immature or coinbase transactions in a special way?

@andrewtoth andrewtoth force-pushed the andrewtoth:track-generated branch from a77eeb4 to 01e1a94 Nov 6, 2018

@andrewtoth

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

You're absolutely right. I didn't check for having the server running while receiving a coinbase transaction. I've updated the commit to check for coinbase transactions in check_for_new_txes.

I did test this with Electrum wallet. However, I haven't had the server running while receiving a coinbase. I can't think of a way it would break since the logic is handled the same as when starting up, but that is definitely something to think about. Electrum displays these transactions normally, except when immature it displays a separate balance in brackets next to the main balance indicating how much is not yet matured.

Also, I opened a PR bitcoin/bitcoin#14653 to fix the help text for listtransactions rpc. I believe that could have prevented this.

@chris-belcher chris-belcher force-pushed the chris-belcher:master branch from 5defe1b to e8b48e4 Nov 6, 2018

@chris-belcher chris-belcher merged commit 6e82d6f into chris-belcher:master Nov 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.