refactor: Get rid of more redundant chain methods #19425

ryanofsky · 2020-07-01T16:54:33Z

This just drops three interfaces::Chain methods replacing them with other calls.

Motivation for removing these chain methods:

Need to get rid of findFirstBlockWithTimeAndHeight for Multiprocess bitcoin #10102, which doesn't support overloaded methods
Followup from Reverse cs_main, cs_wallet lock order and reduce cs_main locking #16426 (comment)
phantomcircuit comments about findNextBlock test http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-06.html#l-214

Behavior is not changing in any way here. A TODO comment in ScanForWalletTransactions was removed, but just because it was invalid (see #19195 (comment)), not because it was implemented.

ryanofsky · 2020-07-01T17:36:34Z

Updated 0c9dea5 -> 87bc761 (pr/next.1 -> pr/next.2, compare) adding comments and removing stale TODO. Still no changes in behavior
Updated 87bc761 -> 91ac038 (pr/next.2 -> pr/next.3, compare) with more minor cleanups

DrahtBot · 2020-07-01T17:44:24Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

Drop some TSan suppressions #19983 (Drop some TSan suppressions by hebasto)
test: Fix inconsistent lock order in wallet_tests/CreateWallet #19982 (test: Fix inconsistent lock order in wallet_tests/CreateWallet by hebasto)
wallet: ScanForWalletTransactions cleanup #19195 (wallet: ScanForWalletTransactions cleanup by pstratem)
multiprocess: Add basic spawn and IPC support #19160 (multiprocess: Add basic spawn and IPC support by ryanofsky)

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.

promag

Concept ACK.

laanwj · 2020-07-09T16:45:11Z

Concept ACK. I find this code really hard to review.

src/interfaces/chain.cpp

promag · 2020-07-16T22:05:38Z

src/interfaces/chain.cpp

@@ -46,6 +46,8 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec
    if (block.m_time) *block.m_time = index->GetBlockTime();
    if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax();
    if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast();
+    if (block.m_in_active_chain) *block.m_in_active_chain = ::ChainActive()[index->nHeight] == index;
+    if (block.m_next_block) FillBlock(ChainActive()[index->nHeight] == index ? ChainActive()[index->nHeight + 1] : nullptr, *block.m_next_block, lock);
    if (block.m_data) {
        REVERSE_LOCK(lock);
        if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();


Unrelated, but why return true if ReadBlockfromDisk fails?

re: #19425 (comment)

Unrelated, but why return true if ReadBlockfromDisk fails?

findBlock and related methods return true if the block is found, false if it is not found. Whether block data is available is a different question. In practice ReadBlockFromDisk fails when a block is pruned, and callers that want to see if data is pruned can check data.IsNull. We could also add more accessors to return pruning information if there's a need.

Suggested bitcoin#19425 (comment)

ryanofsky

Updated 91ac038 -> 7ed165a (pr/next.3 -> pr/next.4, compare) adding commit to get rid of ChainActive

ryanofsky · 2020-07-16T23:03:27Z

src/interfaces/chain.cpp

@@ -46,6 +46,8 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec
    if (block.m_time) *block.m_time = index->GetBlockTime();
    if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax();
    if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast();
+    if (block.m_in_active_chain) *block.m_in_active_chain = ::ChainActive()[index->nHeight] == index;
+    if (block.m_next_block) FillBlock(ChainActive()[index->nHeight] == index ? ChainActive()[index->nHeight + 1] : nullptr, *block.m_next_block, lock);
    if (block.m_data) {
        REVERSE_LOCK(lock);
        if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull();


re: #19425 (comment)

Unrelated, but why return true if ReadBlockfromDisk fails?

findBlock and related methods return true if the block is found, false if it is not found. Whether block data is available is a different question. In practice ReadBlockFromDisk fails when a block is pruned, and callers that want to see if data is pruned can check data.IsNull. We could also add more accessors to return pruning information if there's a need.

src/interfaces/chain.cpp

laanwj · 2020-09-01T12:33:59Z

I managed to review the code and am fairly sure it is correct.
Code review ACK 6e10c2b

Suggested bitcoin#19425 (comment)

ryanofsky · 2020-09-28T11:04:01Z

Rebased 6e10c2b -> 9931714 (pr/next.5 -> pr/next.6, compare) due to conflict with #19619

dongcarl · 2020-10-19T20:47:53Z

Code Review ACK 9931714

97e683d doesn't seem to change behaviour, and seems to match FoundBlock's conventions. I found the calling conventions to be a bit weird but they're still understandable. Perhaps someone can tell me why we do it this way, is it for efficiency?
9931714 arrived at the same conclusion as I did for resolving multiple NodeContexts. See: ef1b08c (#20158) and a9539e7 (#20158). Elimination of ::ChainActive() also makes my diff for tree-wide: De-globalize ChainstateManager #20158 shorter.

ryanofsky · 2020-10-19T21:07:13Z

Thanks for reviewing!

Perhaps someone can tell me why we do it this way, is it for efficiency?

It's called a "fluent interface" and it's just a way emulating a way of named parameters in languages which don't support them: https://en.wikipedia.org/wiki/Fluent_interface. The alternatives are adding unnamed parameters or adding method overloads

maflcko

cr ACK 9931714 🍲

Show signature and timestamp

Signature:

python: error while loading shared libraries: libpython3.8.so.1.0: cannot open shared object file: No such file or directory

maflcko · 2020-11-19T08:30:23Z

src/interfaces/chain.h

@@ -44,6 +44,10 @@ class FoundBlock
    FoundBlock& time(int64_t& time) { m_time = &time; return *this; }
    FoundBlock& maxTime(int64_t& max_time) { m_max_time = &max_time; return *this; }
    FoundBlock& mtpTime(int64_t& mtp_time) { m_mtp_time = &mtp_time; return *this; }
+    //! Return whether block is in the active (most-work) chain.
+    FoundBlock& inActiveChain(bool& in_active_chain) { m_in_active_chain = &in_active_chain; return *this; }
+    //! Return next block in the active chain if current block is in the active chain.


It seems fragile to require the caller to have to pass inActiveChain here as well and initialize the bool reference that was passed to false.

At the very least the dev docs should mention the footgun.

Curious: Why does the passed bool reference have to be initialized to false? Wouldn't it be okay for it to be initialized to true as well?

That would imply if the block isn't found it is assumed to be in the active chain? Seems a footgun just as dangerous

re: #19425 (comment)

That would imply if the block isn't found it is assumed to be in the active chain? Seems a footgun just as dangerous

Information about a block isn't returned if the block doesn't exist. Maybe this is a footgun but it is also pre-existing behavior and how every other accessor works here. I don't think this PR would be improved by initializing this one property while not initializing the others. Inconsistent initialization seems like a bigger footgun, worse than uniformly requiring callers to initialize values that they read.

Maybe in the future another PR could decide on special default values to return for each block property when a block doesn't exist. Maybe this would make calling code safer in case it forgets to initialize a variable. But I could also imagine it leading to other bugs if a caller choses its own different default value and then checks against that wrong value. I could imagine it making calling code harder to read, because you may need to look up default values to understand it instead of being able to rely on initial values being unchanged. All of this is arguable and I don't have strong opinions, but I do think changing this would go beyond scope of current PR and that it would probably be better to look into separately.

maflcko · 2020-11-19T09:11:45Z

src/bench/wallet_balance.cpp

-    NodeContext node;
-    std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
-    CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
+    test_setup.m_node.chain = interfaces::MakeChain(test_setup.m_node);


Can you explain what this is doing and why the redundant call is needed?

diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 98eac80055..aa436ee3ea 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -24,7 +24,6 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE; - test_setup.m_node.chain = interfaces::MakeChain(test_setup.m_node); CWallet wallet{test_setup.m_node.chain.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp index b8c4bc8605..73463b071e 100644 --- a/src/test/interfaces_tests.cpp +++ b/src/test/interfaces_tests.cpp @@ -17,7 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup) BOOST_AUTO_TEST_CASE(findBlock) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); uint256 hash; @@ -61,7 +61,7 @@ BOOST_AUTO_TEST_CASE(findBlock) BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); uint256 hash; int height; @@ -73,7 +73,7 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight) BOOST_AUTO_TEST_CASE(findAncestorByHeight) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); uint256 hash; BOOST_CHECK(chain->findAncestorByHeight(active[20]->GetBlockHash(), 10, FoundBlock().hash(hash))); @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight) BOOST_AUTO_TEST_CASE(findAncestorByHash) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); int height = -1; BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height))); @@ -93,7 +93,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash) BOOST_AUTO_TEST_CASE(findCommonAncestor) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); auto* orig_tip = active.Tip(); for (int i = 0; i < 10; ++i) { @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor) BOOST_AUTO_TEST_CASE(hasBlocks) { - auto chain = interfaces::MakeChain(m_node); + auto& chain = m_node.chain; const CChain& active = Assert(m_node.chainman)->ActiveChain(); // Test ranges diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f38ccba384..4127cd45f8 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Make sure that can use BnB when there are preset inputs empty_wallet(); { - std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), "", CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); wallet->SetupLegacyScriptPubKeyMan(); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index c80310045a..334e4ae0d8 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -10,7 +10,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args)); + m_wallet_client = MakeWalletClient(*m_node.chain, *Assert(m_node.args)); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h index f5bade77df..f666c45a34 100644 --- a/src/wallet/test/init_test_fixture.h +++ b/src/wallet/test/init_test_fixture.h @@ -19,7 +19,6 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup { fs::path m_datadir; fs::path m_cwd; std::map<std::string, fs::path> m_walletdir_path_cases; - std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); std::unique_ptr<interfaces::WalletClient> m_wallet_client; }; diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index d5aed99d99..0ef8b9c4bf 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -27,8 +27,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) CKey uncompressedKey; uncompressedKey.MakeNewKey(false); CPubKey uncompressedPubkey = uncompressedKey.GetPubKey(); - NodeContext node; - std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node); + std::unique_ptr<interfaces::Chain>& chain = m_node.chain; CScript scriptPubKey; isminetype result; diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp index f7c1337b0d..347a436429 100644 --- a/src/wallet/test/scriptpubkeyman_tests.cpp +++ b/src/wallet/test/scriptpubkeyman_tests.cpp @@ -17,9 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(scriptpubkeyman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(CanProvide) { // Set up wallet and keyman variables. - NodeContext node; - std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan(); // Make a 1 of 2 multisig script diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 4d6f427618..badf2eb459 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -6,10 +6,10 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), - m_wallet(m_chain.get(), "", CreateMockWalletDatabase()) + m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase()) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); - m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); + m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_wallet_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index ba8a5ff1f3..ab7fb8c42b 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -20,8 +20,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); - std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args)); + std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_node.chain, *Assert(m_node.args)); CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 156be053d0..eb0bbb6ccc 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -83,8 +83,6 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex* newTip = ::ChainActive().Tip(); - m_node.chain = interfaces::MakeChain(m_node); - // Verify ScanForWalletTransactions fails to read an unknown start block. { CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); @@ -182,8 +180,6 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex* newTip = ::ChainActive().Tip(); - m_node.chain = interfaces::MakeChain(m_node); - // Prune the older block file. { LOCK(cs_main); @@ -253,8 +249,6 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) SetMockTime(KEY_TIME); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); - m_node.chain = interfaces::MakeChain(m_node); - std::string backup_file = (GetDataDir() / "wallet.backup").string(); // Import key into wallet and call dumpwallet to create backup file. @@ -489,7 +483,6 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - m_node.chain = interfaces::MakeChain(m_node); wallet = MakeUnique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); { LOCK2(wallet->cs_wallet, ::cs_main); @@ -701,7 +694,6 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { // Create new wallet with known key and unload it. - m_node.chain = interfaces::MakeChain(m_node); auto wallet = TestLoadWallet(*m_node.chain); CKey key; key.MakeNewKey(true); @@ -794,8 +786,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { - auto chain = interfaces::MakeChain(m_node); - auto wallet = TestLoadWallet(*chain); + auto wallet = TestLoadWallet(*m_node.chain); CKey key; key.MakeNewKey(true); AddKey(*wallet, key);

Right, looks like anything that has a fixture which inherits from BasicTestingSetup will have m_node.chain properly initialized:

bitcoin/src/test/util/setup_common.cpp

Line 109 in a0489f3

m_node.chain = interfaces::MakeChain(m_node);

re: #19425 (comment)

Right, looks like anything that has a fixture which inherits from BasicTestingSetup will have m_node.chain properly initialized:

bitcoin/src/test/util/setup_common.cpp

Line 109 in a0489f3

m_node.chain = interfaces::MakeChain(m_node);

Thanks, removed this and included Marco's patch in new commit 5baa88f. This code in commit 6965f13 was written July and had to call MakeChain because the change which started setting m_node.chain (#19098) was only merged in August.

This just drops three interfaces::Chain methods replacing them with other calls. Motivation for removing these chain methods: - Need to get rid of findFirstBlockWithTimeAndHeight for bitcoin#10102, which doesn't support overloaded methods - Followup from bitcoin#16426 (comment) - phantomcircuit comments about findNextBlock test http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-06.html#l-214 Behavior is not changing in any way here. A TODO comment in ScanForWalletTransactions was removed, but just because it was invalid (see bitcoin#19195 (comment)), not because it was implemented.

Suggested bitcoin#19425 (comment)

These calls are no longer needed after edc3160 from bitcoin#19098 which started instantiating BasicTestingSetup.m_node.chain Patch from MarcoFalke <falke.marco@gmail.com> in bitcoin#19425 (comment) Co-authored-by: MarcoFalke <falke.marco@gmail.com>

Suggested bitcoin#19425 (comment)

ryanofsky

Rebased 9931714 -> 5baa88f (pr/next.6 -> pr/next.7, compare) due to conflict with #20494 and removed MakeChain calls no longer needed after #19098 as suggested

ryanofsky · 2020-12-07T23:36:04Z

src/interfaces/chain.h

@@ -44,6 +44,10 @@ class FoundBlock
    FoundBlock& time(int64_t& time) { m_time = &time; return *this; }
    FoundBlock& maxTime(int64_t& max_time) { m_max_time = &max_time; return *this; }
    FoundBlock& mtpTime(int64_t& mtp_time) { m_mtp_time = &mtp_time; return *this; }
+    //! Return whether block is in the active (most-work) chain.
+    FoundBlock& inActiveChain(bool& in_active_chain) { m_in_active_chain = &in_active_chain; return *this; }
+    //! Return next block in the active chain if current block is in the active chain.


re: #19425 (comment)

That would imply if the block isn't found it is assumed to be in the active chain? Seems a footgun just as dangerous

Information about a block isn't returned if the block doesn't exist. Maybe this is a footgun but it is also pre-existing behavior and how every other accessor works here. I don't think this PR would be improved by initializing this one property while not initializing the others. Inconsistent initialization seems like a bigger footgun, worse than uniformly requiring callers to initialize values that they read.

Maybe in the future another PR could decide on special default values to return for each block property when a block doesn't exist. Maybe this would make calling code safer in case it forgets to initialize a variable. But I could also imagine it leading to other bugs if a caller choses its own different default value and then checks against that wrong value. I could imagine it making calling code harder to read, because you may need to look up default values to understand it instead of being able to rely on initial values being unchanged. All of this is arguable and I don't have strong opinions, but I do think changing this would go beyond scope of current PR and that it would probably be better to look into separately.

ryanofsky · 2020-12-07T23:37:16Z

src/bench/wallet_balance.cpp

-    NodeContext node;
-    std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
-    CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
+    test_setup.m_node.chain = interfaces::MakeChain(test_setup.m_node);


re: #19425 (comment)

Right, looks like anything that has a fixture which inherits from BasicTestingSetup will have m_node.chain properly initialized:

bitcoin/src/test/util/setup_common.cpp

Line 109 in a0489f3

m_node.chain = interfaces::MakeChain(m_node);

Thanks, removed this and included Marco's patch in new commit 5baa88f. This code in commit 6965f13 was written July and had to call MakeChain because the change which started setting m_node.chain (#19098) was only merged in August.

maflcko · 2020-12-08T11:42:40Z

re-ACK 5baa88f 🕶

Show signature and timestamp

Signature:

Timestamp of file with hash 19b54d707dfb2ae462bb723d421d5753739d3d2543051b56a0b9445ec9485431 -

dongcarl · 2020-12-08T17:30:35Z

ACK 5baa88f

ryanofsky force-pushed the pr/next branch from 0c9dea5 to 87bc761 Compare July 1, 2020 17:36

ryanofsky mentioned this pull request Jul 1, 2020

wallet: ScanForWalletTransactions cleanup #19195

Closed

DrahtBot added Refactoring Tests Wallet labels Jul 1, 2020

This was referenced Jul 1, 2020

multiprocess: Add basic spawn and IPC support #19160

Merged

Wallet passive startup #15719

Closed

Multiprocess bitcoin #10102

Draft

promag reviewed Jul 3, 2020

View reviewed changes

This was referenced Jul 7, 2020

multiprocess: Add bitcoin-gui -ipcconnect option #19461

Draft

multiprocess: Add bitcoin-wallet -ipcconnect option #19460

Draft

Prune locks #19463

Draft

ryanofsky force-pushed the pr/next branch from 87bc761 to 91ac038 Compare July 10, 2020 21:49

promag reviewed Jul 16, 2020

View reviewed changes

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Jul 21, 2020

refactor: Replace uses ChainActive() in interfaces/chain.cpp

7ed165a

Suggested bitcoin#19425 (comment)

ryanofsky force-pushed the pr/next branch from 91ac038 to 7ed165a Compare July 21, 2020 15:13

ryanofsky commented Jul 21, 2020

View reviewed changes

This was referenced Jul 22, 2020

refactor: remove ::vpwallets and related global variables #19101

Merged

refactor: Move wallet methods out of chain.h and node.h #19099

Merged

test: Remove duplicate NodeContext hacks #19098

Merged

Use shared pointers only in validation interface #18354

Closed

DrahtBot mentioned this pull request Jul 29, 2020

Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp #19619

Merged

DrahtBot added the Needs rebase label Aug 7, 2020

maflcko added interfaces and removed Refactoring Tests Wallet labels Aug 7, 2020

DrahtBot removed the Needs rebase label Aug 7, 2020

DrahtBot mentioned this pull request Aug 20, 2020

wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets #19077

Merged

DrahtBot added the Needs rebase label Sep 7, 2020

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Sep 25, 2020

refactor: Replace uses ChainActive() in interfaces/chain.cpp

cc4faf1

Suggested bitcoin#19425 (comment)

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Sep 25, 2020

refactor: Replace uses ChainActive() in interfaces/chain.cpp

9931714

Suggested bitcoin#19425 (comment)

ryanofsky force-pushed the pr/next branch from 6e10c2b to 9931714 Compare September 25, 2020 14:49

DrahtBot removed the Needs rebase label Sep 25, 2020

This was referenced Sep 25, 2020

Drop some TSan suppressions #19983

Merged

test: Fix inconsistent lock order in wallet_tests/CreateWallet #19982

Merged

DrahtBot mentioned this pull request Oct 15, 2020

tree-wide: De-globalize ChainstateManager #20158

Closed

12 tasks

DrahtBot mentioned this pull request Nov 5, 2020

tests: Create or use existing properly initialized NodeContexts #20323

Merged

maflcko added the Refactoring label Nov 19, 2020

maflcko approved these changes Nov 19, 2020

View reviewed changes

DrahtBot added the Needs rebase label Dec 1, 2020

ryanofsky and others added 3 commits December 7, 2020 09:09

refactor: Replace uses ChainActive() in interfaces/chain.cpp

6965f13

Suggested bitcoin#19425 (comment)

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Dec 8, 2020

refactor: Replace uses ChainActive() in interfaces/chain.cpp

a85f9b5

Suggested bitcoin#19425 (comment)

ryanofsky force-pushed the pr/next branch from 9931714 to 5baa88f Compare December 8, 2020 02:06

ryanofsky commented Dec 8, 2020

View reviewed changes

DrahtBot removed the Needs rebase label Dec 8, 2020

maflcko merged commit e98d1d6 into bitcoin:master Dec 8, 2020

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 8, 2020

Merge bitcoin#19425: refactor: Get rid of more redundant chain methods

02ed555

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Get rid of more redundant chain methods #19425

refactor: Get rid of more redundant chain methods #19425

ryanofsky commented Jul 1, 2020 •

edited

Loading

ryanofsky commented Jul 1, 2020 •

edited

Loading

DrahtBot commented Jul 1, 2020 •

edited

Loading

promag left a comment

laanwj commented Jul 9, 2020

promag Jul 16, 2020

ryanofsky Jul 16, 2020

ryanofsky left a comment

ryanofsky Jul 16, 2020

laanwj commented Sep 1, 2020

ryanofsky commented Sep 28, 2020

dongcarl commented Oct 19, 2020

ryanofsky commented Oct 19, 2020

maflcko left a comment

maflcko Nov 19, 2020

dongcarl Dec 3, 2020

maflcko Dec 3, 2020

ryanofsky Dec 7, 2020

maflcko Nov 19, 2020

dongcarl Dec 3, 2020

ryanofsky Dec 7, 2020

ryanofsky left a comment

ryanofsky Dec 7, 2020

ryanofsky Dec 7, 2020

maflcko commented Dec 8, 2020

dongcarl commented Dec 8, 2020

refactor: Get rid of more redundant chain methods #19425

refactor: Get rid of more redundant chain methods #19425

Conversation

ryanofsky commented Jul 1, 2020 • edited Loading

ryanofsky commented Jul 1, 2020 • edited Loading

DrahtBot commented Jul 1, 2020 • edited Loading

Conflicts

promag left a comment

Choose a reason for hiding this comment

laanwj commented Jul 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laanwj commented Sep 1, 2020

ryanofsky commented Sep 28, 2020

dongcarl commented Oct 19, 2020

ryanofsky commented Oct 19, 2020

maflcko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maflcko commented Dec 8, 2020

dongcarl commented Dec 8, 2020

ryanofsky commented Jul 1, 2020 •

edited

Loading

ryanofsky commented Jul 1, 2020 •

edited

Loading

DrahtBot commented Jul 1, 2020 •

edited

Loading