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

[Bug] wrong reserveKey when committing budget/proposal collaterals #1855

Conversation

random-zebra
Copy link

The CReserveKey passed to GetBudgetFinalizationCollateralTX/GetBudgetSystemCollateralTX (for the change address) is not the same that is later passed to CommitTransaction.
So, if commit fails, for whatever reason, the original key is not returned to the keypool.

Also, the implementations of GetBudgetFinalizationCollateralTX and GetBudgetSystemCollateralTX are absolutely identical. The only difference is a single constant variable (the fee amount used).
Unify them in CreateBudgetFeeTX (passing a bool fFinalization to select the correct proposal/finalized-budget fee amount).

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK 9cc878a

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

utACK 9cc878a

@random-zebra
Copy link
Author

merging...

@random-zebra random-zebra merged commit 3cd5277 into PIVX-Project:master Sep 17, 2020
furszy added a commit that referenced this pull request Sep 24, 2020
2d10e2e [Trivial][UI] Fix init messages (random-zebra)
3e8ccf0 missing mnping category added to logcategories (furszy)
f182bc8 [Cleanup] emplace CRecipient + remove extra hashing of budget objects (random-zebra)
6204902 [Refactor] Unify GetBudgetSystemCollTX and GetBudgetFinalizationCollTX (random-zebra)
8398b2e [BUG] Duplicated ReserveKey in CBudgetManager::SubmitFinalBudget() (random-zebra)
9c0d4b1 Remove unused mapRequest tracking. (furszy)
a7b3b4f miner: removing a not needed block requests tracking. (furszy)
921fc4b GUI: removing unused TransactionDesc file (static toHTML function is not being called anymore). (furszy)
d956349 GUI: transactionTablemodel, remove unused LongDescriptionRole. (furszy)

Pull request description:

  Simple back porting PR to 4.3 branch, including:

  #1853
  #1855
  #1860
  #1863

ACKs for top commit:
  random-zebra:
    utACK 2d10e2e
  Fuzzbawls:
    utACK 2d10e2e

Tree-SHA512: b23eabfc1ba1c2896782fe77b25d8edad2e0ec6dc0f3a9e78add257e4cf801605bf0f1943690bc04f70587b6d9a8d179770dd312eec40bbfa605bb3caf20cc05
@Fuzzbawls Fuzzbawls modified the milestones: 5.0.0, 4.3.0 Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants