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

[Refactor] Move CBlockFileInfo and CValidationState out of main #1302

Merged

Conversation

Fuzzbawls
Copy link
Collaborator

This is the first part of #1209 that moves the CBlockFileInfo and CValidationState classes (and their supporting constants) out of main.cpp(.h)

@barrystyle's original commits have been further split to assist in review-ability ease.

I left the refactor of nMaxMoneyOut and CBlockTemplate included here as they are rather straight forward, however they each now are in their own respective commits.

@Fuzzbawls Fuzzbawls added this to the 4.1.0 milestone Jan 24, 2020
@Fuzzbawls Fuzzbawls self-assigned this Jan 24, 2020
Copy link

@barrystyle barrystyle left a comment

Choose a reason for hiding this comment

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

@random-zebra mentioned it might be a good idea to group the change in amount.cpp, with the associated QT changes in a separate PR?

@random-zebra
Copy link

Ref: #1209 (comment)

My nit was exclusively about extern const std::string CURRENCY_UNIT which is defined here (in amount.*) but never actually used. It should be used to replace many instances of the explicit string "PIV" in the code, so I suggest we define it when this is addressed (rather than having unused extern variables in the meantime).

@Fuzzbawls
Copy link
Collaborator Author

Removed the unused const for now. it can be added back in if/when it is actually used/connected to the GUI

@random-zebra
Copy link

Also needs rebase due to conflicts with #1290 just merged.

First part of splitting up main.cpp(.h), this moves some common
consensus constants into a new consensus.h file.
The commit following this one abstracts out the CValidationState class
to consensus/validation.h. During that refactor, the class' Abort method
 is removed.

This commit expands on the pre-existing AbortNode method to allow for
the validation state to be passed in.
@Fuzzbawls
Copy link
Collaborator Author

rebased and addressed @random-zebra 's comment

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK f578a68

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 f578a68

LogPrintf("*** %s\n", strMessage);
uiInterface.ThreadSafeMessageBox(
userMessage.empty() ? _("Error: A fatal internal error occured, see debug.log for details") : userMessage,
"", CClientUIInterface::MSG_ERROR);
Copy link

Choose a reason for hiding this comment

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

The second param here is the dialog title, would be good to set it and not leave it empty. Maybe: "Internal Error".

(Stating it for a subsequent PR, not this one.)

@random-zebra
Copy link

Merging...

random-zebra added a commit that referenced this pull request Jan 30, 2020
…of main

f578a68 Move CValidationState to consensus subdir (Fuzzbawls)
083cc4d Use AbortNode instead of state.Abort (Fuzzbawls)
ad77d3d Move consensus constants to consensus.h (Fuzzbawls)
c210d20 Move CBlockTemplate from main.h to miner.h (Fuzzbawls)
4d8b9ce Move CBlockFileInfo class from main.h to chain.h (Fuzzbawls)
668ac61 Refactor nMaxMoneyOut back to amount.h (Fuzzbawls)

Pull request description:

  This is the first part of #1209 that moves the CBlockFileInfo and CValidationState classes (and their supporting constants) out of `main.cpp(.h)`

  @barrystyle's original commits have been further split to assist in review-ability ease.

  I left the refactor of `nMaxMoneyOut` and `CBlockTemplate` included here as they are rather straight forward, however they each now are in their own respective commits.

ACKs for top commit:
  random-zebra:
    utACK f578a68
  furszy:
    utACK f578a68

Tree-SHA512: 4433aacde02be0479d6c8ac3d6d4d725c4ed5518c82a412539e791f62b63516f042828c0407a697b4018c8f92965425f05837293a38d6da890a98bdf0e4e9dfe
@random-zebra random-zebra merged commit f578a68 into PIVX-Project:master Jan 30, 2020
random-zebra added a commit that referenced this pull request Aug 12, 2020
5147571 Break circular dependency main ↔ txdb (random-zebra)
2725fac txdb: Add Cursor() method to CCoinsView to iterate over UTXO set (random-zebra)

Pull request description:

  Other two quick backports needed for the upcoming per-txout model.

  - bitcoin#7756 - Add cursor to iterate over utxo set, use this in `gettxoutsetinfo`
    > - Add a method `Cursor()` to `CCoinsView` that returns a cursor which can be used to iterate over the whole UTXO set.
    > - Implement `GetUTXOStats` in terms of this, remove `GetStats()` method on `CCoinsView`.
    > - Change `gettxoutsetinfo` RPC to use new `GetUTXOStats` function.

  - bitcoin#7815 - Break circular dependency main ↔ txdb (part of this was done already in #1302 and #1319)
    > - Moving `CBlockFileInfo` from main.h to chain.h. I think this makes sense, as the other block-file stuff is there too.
    > - Moving `CDiskTxPos` from main.h to txdb.h. This type, used for `-txindex` seems specific to txdb.
    > - Pass a functor `insertBlockIndex` to `LoadBlockIndexGuts`. This leaves it up to the caller how to insert block indices.

ACKs for top commit:
  furszy:
    ACK 5147571
  Fuzzbawls:
    ACK 5147571

Tree-SHA512: 166f5d797fac68667c59e002a3596af65596cd6ebee5e8b4bee538e6f66ec5f6365e4dd0bbb44be2226ebb51411d4db01f9f2417660841ed7a5125b3c8a7ad97
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.

4 participants