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

[Refactoring] Budget, round 5: proposals/budgets FeeTX indexes #1858

Merged

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Sep 18, 2020

Based on top of:

Starting with [Budget] Introduce mapFeeTxToProposal and mapFeeTxToBudget (0eb9e45)

Since #1845 we no longer check repeatedly the active chain, when updating a budget object, we just save the height of the block including the collateral tx (ref #1845), when we first add the proposal/budget to the map.

Therefore we need to handle the case where said block is disconnected from the chain.

In order to do so efficiently, this PR adds two indexes (mapFeeTxToBudget/mapFeeTxToProposal), mapping collateral txids to the relative budget objects (while such objects are stored in the map).
A simple function CBudgetManager::RemoveByFeeTxId() can then be called from DisconnectBlock when undoing transactions, in order to remove the now-conflicted budget objects (without performing any expensive search).

@random-zebra
Copy link
Author

Rebased, ready for review.

Keep track of the collateral txid for current proposals/budgets.
Update in AddProposal/AddFinalizedBudget - CheckAndRemove
and database it with CBudgetManager.

Also rename mapCollateralTxids to mapUnconfirmedFeeTx to avoid confusion
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.

nice, code review ACK d7a1c99.

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.

ACK d7a1c99

@random-zebra random-zebra merged commit 951470b into PIVX-Project:master Nov 2, 2020
furszy added a commit that referenced this pull request Nov 6, 2020
…ized budgets

5c0d2aa [Refactor] Fix styling (random-zebra)
bcc7d7c [Cleanup] Remove not needed comparator (random-zebra)
8cf2ef0 [Trivial] Pass references to vote constructors (random-zebra)
56f176a [Trivial] Pass manager object to DumpBudgets (random-zebra)
7a5c02a [Refactor] Don't re-hash proposals in CFinalizedBudget::CheckProposals (random-zebra)
b454d60 [Refactor] GetBudget() return proposal copies instead of pointers (random-zebra)
b6af31c [Cleanup] Don't lock cs_budgets before CBudgetManager::SubmitVote (random-zebra)
cbc6583 [Refactor] Check finalized budget inside CBudgetManager::SubmitVote (random-zebra)
9eceb21 [Refactor] Move finalized budget vote submission to budget-manager (random-zebra)
001f15d [Trivial] Rename global budget object to g_budgetman (random-zebra)

Pull request description:

  Based on top of
  -  [x] #1858

  Starting with `[Trivial] Rename global budget object to g_budgetman` (64f0840)

  This refactors `CFinalizedBudget::SubmitVote`, moving it to the budget manager (`VoteOnFinalizedBudgets`), and removing the nested locking issues.
  It also does some trivial cleanup:
  - rename the global budget manager `g_budgetman`
  - pass const references to  `CBudgetVote`/`CFinalizedBudget` constructors
  - pass the manager externally to `DumpBudgets` (instead of using the global one)

ACKs for top commit:
  furszy:
    utACK 5c0d2aa after rebase for 1916 and merging..

Tree-SHA512: 0d12ca38f177e6fd5b1013f118375ca37a498593076cdc1877f5fb0e9329dd8e7292beee866600bc98b002476b3d9d44dac1d2cb2adbaa7276b52d97c5702119
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