qa: Test shared validation interface #18471

promag · 2020-03-30T16:40:48Z

Based on #18338, only last commit matters. Quoting the test description:

// Test UnregisterSharedValidationInterface ensuring that if interface is
// unregistered during the middle of a callback, interface is destroyed as soon
// as callback returns.

All credits to ryanofsky.

promag · 2020-03-30T16:42:42Z

Initially this was in #18338, decided to push a new PR because some travis job hanged.

DrahtBot · 2020-03-30T18:34:10Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

validation: add const for minimum witness commitment size #18780 (validation: add const for minimum witness commitment size by fanquake)

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.

Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: bitcoin#18517 bitcoin#18471

d6815a2 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky) Pull request description: Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: #18517, #18471 ACKs for top commit: MarcoFalke: ACK d6815a2 laanwj: ACK d6815a2 hebasto: re-ACK d6815a2 promag: ACK d6815a2. Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919

ryanofsky · 2020-04-06T15:34:10Z

I think #18524 (now merged) might fix the xenial test hang: https://travis-ci.org/github/bitcoin/bitcoin/jobs/668847707

The previous code for MainSignalsInstance::Clear would decrement the reference count of every interface, including ones that were already Unregister()ed but still being executed.

promag · 2020-04-07T09:44:56Z

Now based on #18551, applied the following change to the test to account for @sipa fix (this now fails in master):

@@ -397,6 +397,7 @@ BOOST_AUTO_TEST_CASE(release_shared)
     BOOST_CHECK(!test_interface);
     BOOST_CHECK_EQUAL(state, CALLED);
     state = UNREGISTERED;
+    UnregisterAllValidationInterfaces();
     unregistered.set_value(true);
     BOOST_CHECK(destroyed.get_future().get());
     BOOST_CHECK_EQUAL(state, DESTROYED);

promag · 2020-04-07T11:33:23Z

appveyor failed with OSError: [WinError 10048] Only one usage of each socket address (protocol/network address/port) is normally permitted, restarted.

Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: bitcoin#18517 bitcoin#18471

DrahtBot · 2020-04-30T14:03:33Z

🐙 This pull request conflicts with the target branch and needs rebase.

Summary: d6815a2313158862d448733954a73520f223deb6 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky) Pull request description: Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: bitcoin/bitcoin#18517, bitcoin/bitcoin#18471 ACKs for top commit: MarcoFalke: ACK d6815a2313158862d448733954a73520f223deb6 laanwj: ACK d6815a2313158862d448733954a73520f223deb6 hebasto: re-ACK d6815a2313158862d448733954a73520f223deb6 promag: ACK d6815a2313158862d448733954a73520f223deb6. Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919 Backport of Core [[bitcoin/bitcoin#18524 | PR18524]] Depends on D6652 Test Plan: `ninja check check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D6653

Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: bitcoin/bitcoin#18517 bitcoin/bitcoin#18471

fanquake · 2021-04-08T06:26:00Z

@promag are you still interested in working on / making this change?

DrahtBot · 2021-12-15T11:22:05Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

DrahtBot · 2022-03-21T13:07:32Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

…rface d6815a2 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky) Pull request description: Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions. Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: bitcoin#18517, bitcoin#18471 ACKs for top commit: MarcoFalke: ACK d6815a2 laanwj: ACK d6815a2 hebasto: re-ACK d6815a2 promag: ACK d6815a2. Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919

DrahtBot added GUI Tests Validation Wallet labels Mar 30, 2020

This was referenced Mar 30, 2020

Build: Move wallet RPCs to their own libbitcoin_walletrpcs module #18469

Closed

Use shared pointers only in validation interface #18354

Closed

Replace -upgradewallet startup option with upgradewallet RPC #15761

Merged

Multiprocess bitcoin #10102

Draft

ryanofsky mentioned this pull request Apr 4, 2020

refactor: drop boost::signals2 in validationinterface #18524

Merged

maflcko closed this Apr 6, 2020

maflcko reopened this Apr 6, 2020

maflcko removed the GUI label Apr 6, 2020

sipa and others added 2 commits April 6, 2020 19:06

Do not clear validationinterface entries being executed

c182fd5

The previous code for MainSignalsInstance::Clear would decrement the reference count of every interface, including ones that were already Unregister()ed but still being executed.

Add RegisterSharedValidationInterface test

c976165

promag force-pushed the 2020-03-test-shared-validation-interface branch from 0e3aeb5 to c976165 Compare April 7, 2020 09:42

promag mentioned this pull request Apr 7, 2020

Do not clear validationinterface entries being executed #18551

Merged

DrahtBot mentioned this pull request Apr 27, 2020

validation: add const for minimum witness commitment size #18780

Merged

maflcko added Needs rebase Waiting for author labels Apr 27, 2020

DrahtBot removed the Needs rebase label Apr 27, 2020

maflcko added Waiting for author and removed Validation Waiting for author Wallet labels Apr 29, 2020

DrahtBot added the Needs rebase label Apr 30, 2020

maflcko removed the Waiting for author label Apr 8, 2021

maflcko added the Up for grabs label Mar 21, 2022

vijaydasmp mentioned this pull request Apr 5, 2022

backport: merge bitcoin #16688 dashpay/dash#4754

Closed

fanquake closed this Apr 26, 2022

bitcoin locked and limited conversation to collaborators Apr 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

qa: Test shared validation interface #18471

qa: Test shared validation interface #18471

Uh oh!

promag commented Mar 30, 2020 •

edited by fanquake

Loading

Uh oh!

promag commented Mar 30, 2020

Uh oh!

DrahtBot commented Mar 30, 2020 •

edited

Loading

Uh oh!

ryanofsky commented Apr 6, 2020

Uh oh!

promag commented Apr 7, 2020

Uh oh!

promag commented Apr 7, 2020 •

edited

Loading

Uh oh!

DrahtBot commented Apr 30, 2020

Uh oh!

fanquake commented Apr 8, 2021

Uh oh!

DrahtBot commented Dec 15, 2021

Uh oh!

DrahtBot commented Mar 21, 2022

Uh oh!

Uh oh!

qa: Test shared validation interface #18471

qa: Test shared validation interface #18471

Uh oh!

Conversation

promag commented Mar 30, 2020 • edited by fanquake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Mar 30, 2020

Uh oh!

DrahtBot commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

ryanofsky commented Apr 6, 2020

Uh oh!

promag commented Apr 7, 2020

Uh oh!

promag commented Apr 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Apr 30, 2020

Uh oh!

fanquake commented Apr 8, 2021

Uh oh!

DrahtBot commented Dec 15, 2021

Uh oh!

DrahtBot commented Mar 21, 2022

Uh oh!

Uh oh!

promag commented Mar 30, 2020 •

edited by fanquake

Loading

DrahtBot commented Mar 30, 2020 •

edited

Loading

promag commented Apr 7, 2020 •

edited

Loading