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.

Sign up for GitHub

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

Jump to bottom

Better coinbase handling in wallet #4353

Closed
SWvheerden opened this issue Jul 28, 2022 · 0 comments · Fixed by #4386
Closed

Better coinbase handling in wallet #4353

SWvheerden opened this issue Jul 28, 2022 · 0 comments · Fixed by #4386
Assignees
@jorgeantonio21

Comments

@SWvheerden
Copy link
Collaborator

The wallet generates new coinbases for mines when asked. This is done on the height and amount.

What the wallet does is when it is asked for a new coinbase it searches the db for a coinbase tx for that height and amount. If a match is found, that is returned. If a match is not found, it creates a new tx for that height and amount. But it also delete's all other coinbases for that height.

This is a problem as some miner might still be mining that coinbase tx. The wallet should not just right of the bat delete all other coinbases.

The other problem is that the miner generates an offset_private key out of random each time. This should be changed to a derived key(might use the current coinbase_pvt key) so that it is the same.
It could happen if two miners ask for a new coinbase very close to each other that the wallet generates two coinbases only keeps one. Then the miner who won the block has its coinbases deleted, but the wallet thinks it has it due to the fact that the commitment matches.
@agubarev agubarev assigned agubarev and unassigned agubarev Aug 3, 2022
@jorgeantonio21 jorgeantonio21 mentioned this issue Aug 4, 2022
Merged
@jorgeantonio21 jorgeantonio21 self-assigned this Aug 5, 2022
aviator-app bot pushed a commit that referenced this issue Aug 9, 2022
@jorgeantonio21
fix: better coinbase handling (see issue #4353) (#4386)
5581044 
Description
--- 
The current coinbase handling relies on height and amount. If a match is not found for that height and amount, then it creates a new tx, deleting all the existing coinbases for that height. This is a problem, as miners can be working concurrently on a (to-be deleted) coinbase. This PR addresses this issue (see #4353).

Motivation and Context
--- 
Fixes #4353.

How Has This Been Tested?
--- 
Unit and cucumber tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jorgeantonio21 jorgeantonio21

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: better coinbase handling (see issue #4353) jorgeantonio21/tari
3 participants
@agubarev @SWvheerden @jorgeantonio21