Skip to content

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Jul 16, 2023

@kwvg kwvg force-pushed the assumeutxo4 branch 2 times, most recently from 1dcd32d to b7ff846 Compare July 18, 2023 23:45
@kwvg kwvg changed the title backport: merge bitcoin#21391, #19550, #15946, #21230, #21252, #21297, #20605, #21575 ,#22309, #19521, #21796, #21767, #21526 (assumeutxo backports: part 4) backport: merge bitcoin#21391, #19550, #15946, #21230, #21252, #21297, #20605, #21575, #22309, #19521, #19762, #21796 ,#21767 (assumeutxo backports: part 4) Jul 18, 2023
@kwvg kwvg force-pushed the assumeutxo4 branch 7 times, most recently from 7c2eb36 to 6826dea Compare July 23, 2023 17:44
@kwvg kwvg changed the title backport: merge bitcoin#21391, #19550, #15946, #21230, #21252, #21297, #20605, #21575, #22309, #19521, #19762, #21796 ,#21767 (assumeutxo backports: part 4) backport: merge bitcoin#21391, #19550, #15946, #21230, #21252, #21297, #20605, #21575, #22309, #19762 (assumeutxo backports: part 4) Jul 23, 2023
@github-actions
Copy link

This pull request has conflicts, please rebase.

@github-actions
Copy link

This pull request has conflicts, please rebase.

@UdjinM6
Copy link

UdjinM6 commented Jul 24, 2023

build failed

evo/creditpool.cpp: In function ‘std::optional<CBlock> GetBlockForCreditPool(const CBlockIndex*, const Consensus::Params&)’:
evo/creditpool.cpp:112:10: error: ‘ReadBlockFromDisk’ was not declared in this scope
  112 |     if (!ReadBlockFromDisk(block, block_index, consensusParams)) {
      |          ^~~~~~~~~~~~~~~~~

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

also need to add missing header node/blockstorage.h to evo/creditpool.cpp

self.log.info("check if we can access the blockfilter of a pruned block")
assert(len(self.nodes[1].getblockfilter(self.nodes[1].getblockhash(2))['filter']) > 0)
self.log.info("start node without blockfilterindex")
self.stop_node(1, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add a new variable such as

governance_failure = 'Warning: You are starting with gover....

and use it instead literals?

Same as EXPECTED_STDERR_NO_GOV_PRUNE in test/functional/feature_pruning.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do in a future PR

@kwvg kwvg force-pushed the assumeutxo4 branch 2 times, most recently from 2b189ef to 8316f97 Compare July 24, 2023 18:47
@kwvg kwvg requested a review from knst July 24, 2023 19:17
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

check comment about g_chainman in CreateUTXOSnapshot


Btw for next time better for massive changes such as g_chainman refactoring to create dedicated PR rather than add some extra backports to make "batch of 10" IMO

these 2 are big enough for dedicated PR. I am after meticulous reading still not 100% sure that all changes here are valid, but I haven't found any other issues ATM except mentioned comment.

@kwvg
Copy link
Collaborator Author

kwvg commented Jul 27, 2023

This PR was already split in half due to review complexity concerns, I'd be glad to split it even further if told earlier about it. Referring to them as "extra" backports aren't representative of the reason they're included. If you look at #5501 (which is what this PR originally was before it was split in half), you can see they're the prerequisites for implementing coinstats support which is used by PRs further down the line and bugfixes that result in CI errors due to race conditions.

Work on g_chainman deglobalization is purely incidental and has arisen due to it being a dependency for backporting assumeutxo capabilities.

@kwvg kwvg dismissed stale reviews from PastaPastaPasta and UdjinM6 via bdd1b74 July 27, 2023 19:12
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK

@kwvg kwvg requested review from UdjinM6 and PastaPastaPasta July 27, 2023 19:15
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

re-utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

re-utACK

@PastaPastaPasta PastaPastaPasta merged commit d9a0b32 into dashpay:develop Jul 28, 2023
@thephez thephez added the RPC Some notable changes to RPC params/behaviour/descriptions label Aug 1, 2023
thephez added a commit to thephez/docs-core that referenced this pull request Oct 3, 2023
thephez added a commit to dashpay/docs-core that referenced this pull request Oct 3, 2023
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants