Skip to content

Conversation

ryanofsky
Copy link
Contributor

Move ChainstateManager options into m_options struct to simplify class initialization, organize class members, and to name external option variables differently than internal state variables.

This change was originally in #25862, but it was suggested to split off in #25862 (comment) so it could be merged earlier and reduce conflicts with other PRs.

Move ChainstateManager options into m_options struct to simplify class
initialization, organize class members, and to name external option variables
differently than internal state variables.

This change was originally in bitcoin#25862, but it was suggested to split off in
bitcoin#25862 (comment) so it could
be merged earlier and reduce conflicts with other PRs.
@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 23, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25862 (refactor, kernel: Remove gArgs accesses from dbwrapper and txdb by ryanofsky)
  • #25781 (Remove almost all blockstorage globals by MarcoFalke)
  • #25704 (refactor: Remove almost all validation option globals by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@naumenkogs
Copy link
Member

ACK 6db2c00

src/validation.h Outdated
@@ -881,6 +878,7 @@ class ChainstateManager
*/
RecursiveMutex& GetMutex() const LOCK_RETURNED(::cs_main) { return ::cs_main; }

Options m_options;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to remove the const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

re: #25905 (comment)

Any reason to remove the const?

Nope, added const

Copy link
Contributor Author

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Updated 6db2c00 -> 7bc33a8 (pr/csmopt.1 -> pr/csmopt.2, compare) adding const as suggested

src/validation.h Outdated
@@ -881,6 +878,7 @@ class ChainstateManager
*/
RecursiveMutex& GetMutex() const LOCK_RETURNED(::cs_main) { return ::cs_main; }

Options m_options;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

re: #25905 (comment)

Any reason to remove the const?

Nope, added const

@naumenkogs
Copy link
Member

ACK 7bc33a8

@maflcko maflcko merged commit d36bec9 into bitcoin:master Aug 25, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 25, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants