[Bundle 5/n] Prune g_chainman usage in RPC modules #21391

dongcarl · 2021-03-08T21:18:11Z

Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

Based on:

[Bundle 4/n] Prune g_chainman usage in validation-adjacent modules #21270 | [Bundle 4/n] Prune g_chainman usage in validation-adjacent modules
[Bundle 4.5/n] Followup fixups to bundle 4 #21525 | [Bundle 4.5/n] Followup fixups to bundle 4

Note to reviewers:

This bundle may apparently introduce usage of g_chainman or ::Chain(state|)Active() globals, but these are resolved later on in the overall PR. Commits of overall PR
There may be seemingly obvious local references to ChainstateManager or other validation objects which are not being used in callers of the current function in question, this is done intentionally to keep each commit centered around one function/method to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. Commits of overall PR
When changing a function/method that has many callers (e.g. LookupBlockIndex with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add new_function, make old_function a wrapper of new_function, divert all calls to old_function to new_function in the local module only
2. Scripted-diff to divert all calls to old_function to new_function in the rest of the codebase
3. Remove old_function

DrahtBot · 2021-03-09T04:25:27Z

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: UpdateTip/CheckBlockIndex assumeutxo support #21526 (validation: UpdateTip/CheckBlockIndex assumeutxo support by jamesob)
validation: run VerifyDB on all chainstates #21523 (validation: run VerifyDB on all chainstates by jamesob)
Add feerate histogram to getmempoolinfo #21422 (Add feerate histogram to getmempoolinfo by kiminuo)
rpc/validation: enable packages through testmempoolaccept #20833 (rpc/validation: enable packages through testmempoolaccept by glozow)
rpc: Add RPCContext #20017 (rpc: Add RPCContext by promag)
Coinstats Index #19521 (Coinstats Index by fjahr)
Introduce deploymentstatus #19438 (Introduce deploymentstatus by ajtowns)
RPC/Mining: Clean out pre-Segwit miner compatibility code #19391 (RPC/Mining: Clean out pre-Segwit miner compatibility code by luke-jr)

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.

ryanofsky

Code review ACK last 6 commits only a041676. Many chainmans.

384c56d rpc/*,rest: Add review-only assertion to EnsureChainman (14/19)
b6d3847 rpc/blockchain: Use existing NodeContext (15/19)
2b0fb0e rpc/mining: Use existing NodeContext (16/19)
8e3d455 rpc/rawtx: Use existing NodeContext (17/19)
cbe4f7b rest: Pass in NodeContext to rest_block (18/19)
a041676 rest: Use existing NodeContext (19/19)

dongcarl · 2021-03-09T17:40:44Z

Pushed a041676 -> 6b89f53

Fixed assert linter complaint by using CHECK_NONFATAL instead of assert

DrahtBot · 2021-03-15T16:49:01Z

🕵️ @sipa @fjahr have been requested to review this pull request as specified in the REVIEWERS file.

The scripted-diff in the previous commit should have removed all calls to functions like: Ensure(?!Any)$const std::any& (context|ctx)$, so we can remove them now.

Organize local variables/references such that: 1. There is always a `ChainstateManager` reference before any `LOCK(cs_main)`. 2. NodeContext references are used with Ensure*() functions introduced in previous commit where appropriate to avoid duplicate assertions.

In all rest/rpc-related modules, if there are multiple calls to ActiveChain{,State}(), and the calls fall under the same ::cs_main lock, we can simply take a local reference and use/reuse it instead of calling ActiveChain{,State}() again and again.

dongcarl · 2021-04-14T15:20:26Z

Pushed 6d92313 -> 586190f

Addressed [Bundle 5/n] Prune g_chainman usage in RPC modules #21391 (comment) to keep irrational numbers at bay
Addressed jnewbery's reviews relating to taking a reference once and reusing it

ryanofsky

Code review ACK 586190f. Since last review, no changes to existing commits, just some simple new commits added: three new commits renaming std::any Ensure functions (scripted diff commit and manual pre/post commits), and one new commit factoring out a repeated ActiveChain() call made in a loop. Thanks for the updates!

jnewbery

utACK 586190f

It's not a blocker, but I don't like the proliferation of code outside validation holding m_chain. In general, I don't think there's any reason for code outside validation to take a reference to a CChain - it should be able to call a higher abstraction function on chainman to read whatever data it needs.

jnewbery · 2021-04-15T10:13:53Z

src/rpc/blockchain.cpp

        {
+            ChainstateManager& chainman = EnsureChainman(node);


I think it'd be better to move this EnsureChainman() to the top of the function. If chainman doesn't exist, then it's better to immediately exit from the function.

jnewbery · 2021-04-15T10:15:07Z

src/rpc/blockchain.cpp

-            pcursor = std::unique_ptr<CCoinsViewCursor>(::ChainstateActive().CoinsDB().Cursor());
+            CChainState& active_chainstate = chainman.ActiveChainstate();
+            active_chainstate.ForceFlushStateToDisk();
+            pcursor = std::unique_ptr<CCoinsViewCursor>(active_chainstate.CoinsDB().Cursor());


While you're touching this, perhaps change it to use std::make_unique. That's the standard way to set unique pointers now that we're on c++17.

Shouldn't Cursor return a unique_ptr?

Yes, you're right. Cursor() is passing ownership to the caller.

I'm going to mark this comment as resolved since this PR shouldn't make any changes here. I think Cursor()'s return type should be changed in a separate PR.

(unresolved comment to make it easier to see all that needs to be addressed in follow-ups)

jnewbery · 2021-04-15T10:17:18Z

src/rpc/blockchain.cpp

@@ -2342,8 +2391,9 @@ static RPCHelpMan getblockfilter()
    const CBlockIndex* block_index;
    bool block_was_connected;
    {
+        ChainstateManager& chainman = EnsureAnyChainman(request.context);


Again, chainman is a requirement for this function. I think it makes sense to place the assumption at the top of the function.

jnewbery · 2021-04-15T10:28:02Z

src/rpc/mining.cpp

@@ -44,11 +44,12 @@
 * or from the last difficulty change if 'lookup' is nonpositive.
 * If 'height' is nonnegative, compute the estimate at the time when a given block was found.
 */
-static UniValue GetNetworkHashPS(int lookup, int height) {
-    CBlockIndex *pb = ::ChainActive().Tip();
+static UniValue GetNetworkHashPS(int lookup, int height, const CChain& active_chain) {


Even though it's not annotated as such, I think that holding a reference to m_chain and calling methods on it requires holding cs_main. Perhaps it would be a good idea to either:

annotate this function and add an assertion that cs_main is held; or

pass in a &Chainman, then inside the function lock cs_main, get the CChain&, and call the methods on that CChain&.

I think (2) is nicer - best to limit scope of locks as much as possible.

jnewbery · 2021-04-15T10:31:32Z

src/rpc/mining.cpp

    LOCK(cs_main);
-    return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1);
+    return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1, chainman.ActiveChain());


This line is long and difficult to read. Consider something like this:

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 0cef310c50..b60783761d 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -102,7 +102,9 @@ static RPCHelpMan getnetworkhashps() { ChainstateManager& chainman = EnsureAnyChainman(request.context); LOCK(cs_main); - return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1, chainman.ActiveChain()); + const int blocks{!request.params[0].isNull() ? request.params[0].get_int() : 120}; + const int height{!request.params[1].isNull() ? request.params[1].get_int() : -1}; + return GetNetworkHashPS(blocks, height, chainman.ActiveChain()); }, }; }

It almost certainly compiles to the same thing, is much easier to read and reduces the risk of precedence bugs.

jnewbery · 2021-04-15T10:54:48Z

src/rpc/mining.cpp

@@ -111,7 +113,8 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t&

    {
        LOCK(cs_main);
-        IncrementExtraNonce(&block, ::ChainActive().Tip(), extra_nonce);
+        CHECK_NONFATAL(std::addressof(::ChainActive()) == std::addressof(chainman.ActiveChain()));
+        IncrementExtraNonce(&block, chainman.ActiveChain().Tip(), extra_nonce);


For a follow-up: IncrementExtraNonce() doesn't require cs_main. In fact, we could just get a height using chainman.ActiveHeight() and pass that to IncrementExtraNonce().

jnewbery · 2021-04-15T11:38:29Z

src/rpc/rawtransaction.cpp

-        const CTxMemPool& mempool = EnsureMemPool(request.context);
+        NodeContext& node = EnsureAnyNodeContext(request.context);
+        const CTxMemPool& mempool = EnsureMemPool(node);
+        ChainstateManager& chainman = EnsureChainman(node);


I haven't commented on all of these examples, but I really do think readability is helped by placing the Ensure*() assumptions at the top of the rpc methods.

maflcko

I looked at d7824ac 🐃

Show signature and timestamp

Signature:

Timestamp of file with hash d97e47afcf29e46d1bcd8c3ce2c34513261879f42b7516ce21deecae7598dee6 -

maflcko · 2021-04-17T10:22:53Z

src/miner.cpp

 {
    CMutableTransaction tx{*block.vtx.at(0)};
    tx.vout.erase(tx.vout.begin() + GetWitnessCommitmentIndex(block));
    block.vtx.at(0) = MakeTransactionRef(tx);

-    GenerateCoinbaseCommitment(block, WITH_LOCK(::cs_main, assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman)); return blockman.LookupBlockIndex(block.hashPrevBlock)), Params().GetConsensus());
+    WITH_LOCK(::cs_main, assert(g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock) == prev_block));


cced0f4:

I don't understand this commit. Is the goal to remove this assert? If yes, how are new callers protected from accidentally passing the wrong pointer? If no, you'll need to pass blockman again, making this commit useless.

Also, this seems to put more code burden on the caller, duplicating the LookupBlockIndex at each call site.

It might help to specify a goal of the commit here. Is the goal to avoid locking cs_main twice instead of only once? If yes, a simpler solution would be to just pass chainman (from the rpc context) instead of the blockman reference (which needs cs_main to be taken from chainman).

maflcko · 2021-04-17T11:24:59Z

src/rest.cpp

@@ -250,8 +251,9 @@ static bool rest_block(const std::any& context,
    CBlockIndex* tip = nullptr;
    {
        LOCK(cs_main);
-        tip = ::ChainActive().Tip();
-        pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash);
+        ChainstateManager& chainman = EnsureChainman(context);


This may crash the node if the chainman is not available.

terminate called after throwing an instance of 'UniValue'

maflcko

review ACK 586190f 🍯

Show signature and timestamp

Signature:

Timestamp of file with hash 3e1b6d60dc76018210f94a83c678a1e0f9c63606e06fb1b3d34881ee98abc0d8 -

maflcko · 2021-04-17T15:21:16Z

src/rest.cpp

@@ -181,7 +181,7 @@ static bool rest_headers(const std::any& context,
    headers.reserve(count);


6fb65b4: the scripted diff will modify files that are not tracked by git. Could use ... -- $(git grep -l Ensure)

jnewbery · 2021-05-09T15:18:36Z

src/rpc/blockchain.cpp

+    {
+        LOCK(::cs_main);
+        coins_view = &active_chainstate.CoinsDB();
+        blockman = &active_chainstate.m_blockman;


This doesn't require cs_main.

jnewbery · 2021-05-09T15:20:22Z

src/rpc/blockchain.cpp

+    CCoinsView* coins_view;
+    BlockManager* blockman;


It's slightly confusing to take the references from CoinsDB() and m_blockman and take their addresses to make pointers, and then just dereference those pointers. Better just to use references as the local variables.

… modules 7a799c9 index: refactor-only: Reuse CChain ref (Carl Dong) db33cde index: Add chainstate member to BaseIndex (Carl Dong) f4a47a1 bench: Use existing chainman in AssembleBlock (Carl Dong) 91226eb bench: Use existing NodeContext in DuplicateInputs (Carl Dong) e6b4aa6 miner: Pass in chainman to RegenerateCommitments (Carl Dong) 9ecade1 rest: Add GetChainman function and use it (Carl Dong) fc1c282 rpc/blockchain: Use existing blockman in gettxoutsetinfo (Carl Dong) Pull request description: Overall PR: bitcoin#20158 (tree-wide: De-globalize ChainstateManager) The first 2 commits are fixups addressing review for the last bundle: bitcoin#21391 NEW note: 1. I have opened bitcoin#21766 which keeps track of potential improvements where the flaws already existed before the de-globalization work, please post on that issue about these improvements, thanks! Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: jarolrod: ACK 7a799c9 ariard: Code Review ACK 7a799c9 fjahr: re-ACK 7a799c9 MarcoFalke: review ACK 7a799c9 🌠 ryanofsky: Code review ACK 7a799c9. Basically no change since last review except fixed rebase conflicts and a new comment about REST Ensure() jamesob: conditional ACK 7a799c9 ([`jamesob/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai`](https://github.com/jamesob/bitcoin/tree/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai)) Tree-SHA512: 531c00ddcb318817457db2812d9a9d930bc664e58e6f7f1c746350732b031dd624270bfa6b9f49d8056aeb6321d973f0e38e4ff914acd6768edd8602c017d10e

Summary: Also pass in appropriate object to: BIP9SoftForkDescPushBack PR context: De-globalize ChainstateManager This is a backport of [[bitcoin/bitcoin#21391 | core#21391]] bitcoin/bitcoin@d485e81 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D11510

Summary: Also pass in appropriate object to: - GetNetworkHashPS - [gG]enerateBlock{,s} Also: - Misc style/constness changes This is a partial backport of [[bitcoin/bitcoin#21391 | core#21391]] Depends on D11507 bitcoin/bitcoin@60dc05a Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D11511

Summary: Also pass in appropriate object to: - TxToJSON This is a partial backport of [[bitcoin/bitcoin#21391 | core#21391]] bitcoin/bitcoin@7be0671 Depends on D11511 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D11512

Summary: Note: The third commit fixes an issue introduced by the second, when the node context does not have a chainmanager. > rest: Pass in NodeContext to rest_block bitcoin/bitcoin@3f08934 > rest: Use existing NodeContext bitcoin/bitcoin@d7824ac > rest: Add GetChainman function and use it > > This is not the cleanest change but: > > 1. It fixes the erroneous use of RPC's Ensure*() in rest.cpp, which > cause crashes in REST contexts. > > RPC code wraps all calls in a try/except, REST code does not. > Ensure*(), being part of RPC, expects that its throw's will get > caught by a try/except. But if you use Ensure*() in REST code, since > it doesn't have a try/except wrap, a crash will happen. > > 2. It is consistent with other functions like GetMemPool. > > Someone can probably make this a bit prettier. bitcoin/bitcoin@9ecade1 This is a backport of [[bitcoin/bitcoin#21391 | core#21391]] [6 & 7/14] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D11584

Summary: Add alt `Ensure*` functions accepting `NodeContext` and rename the versions accepting a `std::any` `EnsureAny*()`. Note that the changes in rest.cpp are not applicable because of the out-of-sequence backport of commit bitcoin/bitcoin@9ecade1 in D11585 > rpc: Add alt Ensure* functions acepting NodeContext bitcoin/bitcoin@306b1cd > rpc: Add renamed EnsureAny*() functions > > The renaming avoids overloading mistakes arising out of the untyped `std::any` argument. bitcoin/bitcoin@1570c7e > scripted-diff: rest/rpc: Use renamed EnsureAny*() > > ``` > -BEGIN VERIFY SCRIPT- > sed -i -E 's@Ensure([^(]+)($(request\.|)context$)@EnsureAny\1\2@g' \ > -- src/rest.cpp src/rpc/*.cpp > > -END VERIFY SCRIPT- > ``` bitcoin/bitcoin@6fb65b4 > rest/rpc: Remove now-unused old Ensure functions > > The scripted-diff in the previous commit should have removed all calls > to functions like: Ensure(?!Any)$const std::any& (context|ctx)$, so we > can remove them now. bitcoin/bitcoin@038854f This is a backport of [[bitcoin/bitcoin#21391 | core#21391]] [8, 9, 10 & 11/14] Depends on D11584 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D11586

Summary: Organize local variables/references such that: 1. There is always a `ChainstateManager` reference before any `LOCK(cs_main)`. 2. NodeContext references are used with `Ensure*()` functions introduced in previous commit where appropriate to avoid duplicate assertions. This is a backport of [[bitcoin/bitcoin#21391 | core#21391]] [12/14] bitcoin/bitcoin@6a3d192 Depends on D11586 Test Plan: With clang and debug: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D11587

Summary: One of the two occurences in Bitcoin Core was already fixed in Bitcoin ABC. This is a backport of [[bitcoin/bitcoin#21391 | core#21391]] [13/14] bitcoin/bitcoin@f999139 Depends on D11587 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D11588

Summary: In all rest/rpc-related modules, if there are multiple calls to `ActiveChain{,State}()`, and the calls fall under the same `::cs_main` lock, we can simply take a local reference and use/reuse it instead of calling `ActiveChain{,State}()` again and again. This concludes backport of [[bitcoin/bitcoin#21391 | core#21391]] [14/14] bitcoin/bitcoin@586190f Depends on D11588 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D11589

dongcarl mentioned this pull request Mar 8, 2021

tree-wide: De-globalize ChainstateManager #20158

Closed

12 tasks

DrahtBot added Mining P2P RPC/REST/ZMQ Validation labels Mar 8, 2021

This was referenced Mar 9, 2021

BIP 341: Add Speedy Trial activation parameters #21393

Closed

Implement BIP 8 based Speedy Trial activation #21392

Closed

ryanofsky approved these changes Mar 9, 2021

View reviewed changes

dongcarl force-pushed the 2020-10-libbitcoinruntime-v7 branch from a041676 to 6b89f53 Compare March 9, 2021 17:38

This was referenced Mar 10, 2021

Refactor versionbits deployments to avoid potential uninitialized variables #21401

Closed

Genericide BIP9 in variable/type names and comments #21399

Closed

[rfc] add option to bypass contextual timelocks in testmempoolaccept? #21413

Closed

maflcko removed Mining P2P RPC/REST/ZMQ labels Mar 15, 2021

dongcarl added 5 commits April 14, 2021 11:10

rest/rpc: Remove now-unused old Ensure functions

038854f

The scripted-diff in the previous commit should have removed all calls to functions like: Ensure(?!Any)$const std::any& (context|ctx)$, so we can remove them now.

rpc: Remove unnecessary casting of block height

f999139

rpc: style: Improve BuriedForkDescPushBack signature

bc3bd36

dongcarl force-pushed the 2020-10-libbitcoinruntime-v7 branch from 6d92313 to 586190f Compare April 14, 2021 15:18

DrahtBot mentioned this pull request Apr 14, 2021

rpc: Add RPCContext #20017

Closed

ryanofsky approved these changes Apr 14, 2021

View reviewed changes

This was referenced Apr 14, 2021

Add feerate histogram to getmempoolinfo #21422

Closed

RPC/Mining: Clean out pre-Segwit miner compatibility code #19391

Merged

jnewbery reviewed Apr 15, 2021

View reviewed changes

maflcko reviewed Apr 17, 2021

View reviewed changes

maflcko merged commit 0dd7b23 into bitcoin:master Apr 17, 2021

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 17, 2021

Merge bitcoin#21391: [Bundle 5/n] Prune g_chainman usage in RPC modules

4372e26

laanwj mentioned this pull request Apr 22, 2021

multiprocess: Add basic spawn and IPC support #19160

Merged

This was referenced Apr 23, 2021

followups: Various potential improvements arising out of chainman-deglobalizing review #21766

Closed

[Bundle 6/n] Prune g_chainman usage in auxiliary modules #21767

Merged

jnewbery reviewed May 9, 2021

View reviewed changes

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bundle 5/n] Prune g_chainman usage in RPC modules #21391

[Bundle 5/n] Prune g_chainman usage in RPC modules #21391

dongcarl commented Mar 8, 2021 •

edited by jnewbery

DrahtBot commented Mar 9, 2021 •

edited

ryanofsky left a comment

dongcarl commented Mar 9, 2021

DrahtBot commented Mar 15, 2021

dongcarl commented Apr 14, 2021

ryanofsky left a comment

jnewbery left a comment

jnewbery Apr 15, 2021

jnewbery Apr 15, 2021

maflcko Apr 16, 2021

jnewbery Apr 16, 2021

jnewbery Apr 16, 2021

maflcko Apr 17, 2021

jnewbery Apr 15, 2021

jnewbery Apr 15, 2021

jnewbery Apr 15, 2021

jnewbery Apr 15, 2021

jnewbery Apr 15, 2021

maflcko left a comment

maflcko Apr 17, 2021

maflcko Apr 17, 2021

maflcko left a comment

maflcko Apr 17, 2021

jnewbery May 9, 2021

jnewbery May 9, 2021

		@@ -181,7 +181,7 @@ static bool rest_headers(const std::any& context,
		headers.reserve(count);

[Bundle 5/n] Prune g_chainman usage in RPC modules #21391

[Bundle 5/n] Prune g_chainman usage in RPC modules #21391

Conversation

dongcarl commented Mar 8, 2021 • edited by jnewbery

DrahtBot commented Mar 9, 2021 • edited

Conflicts

ryanofsky left a comment

Choose a reason for hiding this comment

dongcarl commented Mar 9, 2021

DrahtBot commented Mar 15, 2021

dongcarl commented Apr 14, 2021

ryanofsky left a comment

Choose a reason for hiding this comment

jnewbery 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

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

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

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

dongcarl commented Mar 8, 2021 •

edited by jnewbery

DrahtBot commented Mar 9, 2021 •

edited