Replace Connman and BanMan globals with NodeContext local #16839

ryanofsky · 2019-09-09T21:32:08Z

This change is mainly a naming / organization change intended to simplify #10102. It:

Renames struct InitInterfaces to struct NodeContext and moves it from
src/init.h to src/node/context.h. This is a cosmetic change intended to make
the point of the struct more obvious.
Gets rid of BanMan and ConnMan globals making them NodeContext members
instead. Getting rid of these globals has been talked about in past as a way
to implement testing and simulations. Making them NodeContext members is a
way of keeping them accessible without the globals.
Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This
better separates node and wallet rpc methods. Node RPC methods should have
access NodeContext, while wallet RPC methods should only have indirect access
to node functionality via interfaces::Chain.
Adds NodeContext& references to interfaces::Chain class and the
interfaces::MakeChain() function. This is needed to access ConnMan and BanMan
instances without the globals.
Gets rid of redundant Node and Chain instances in Qt tests. This is
needed due to the previous MakeChain change, and also makes test setup a
little more straightforward. More cleanup could be done in the future, but it
will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init
code.

ryanofsky · 2019-09-09T21:36:18Z

I'm looking for concept ACKS before detailed review at the moment. Since this a naming and refactoring change, it'd be great if people could take few minutes and skim the diff, and say whether the new code looks ugly or ok, and offer any suggestions. I'm happy to take suggestions and also split this up more.

promag

Concept ACK.

promag · 2019-09-10T00:03:16Z

src/interfaces/chain.cpp

+    {
+        ChainClients& clients = m_node.chain_clients;
+        ChainClients::iterator it = clients.emplace(clients.end(), client);
+        return MakeHandler([&clients, it] { clients.erase(it); });


Is it better to capture member chain_clients than this?

Also, it would be nice to explain that a std::list is used otherwise this .erase would fail.

Is it better to capture member chain_clients than this?

I think a good starting point is to just capture what's needed without unused data or object references. But in this case, capturing this instead of &clients is worse because in addition to capturing unneeded data, it requires the Handler object to be destroyed before the Chain object, and there's no reason to create that kind of error-prone dependency.

Also, it would be nice to explain that a std::list is used otherwise this .erase would fail.

Sure, added comment.

DrahtBot · 2019-09-10T02:30:31Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17261 (Make ScriptPubKeyMan an actual interface and the wallet to have multiple by achow101)
#17167 (Allow whitelisting outgoing connections by luke-jr)
#16895 (External signer multisig support by Sjors)
#16549 ([WIP] UI external signer support (e.g. hardware wallet) by Sjors)
#16546 ([WIP] External signer support - Wallet Box edition by Sjors)
#16528 ([WIP] Native Descriptor Wallets (take 2) by achow101)
#16442 (Serve BIP 157 compact filters by jimpo)
#16224 (gui: Bilingual GUI error messages by hebasto)
#15454 (Remove the automatic creation and loading of the default wallet by achow101)
#12674 (RPC: Support addnode onetry without making the connection priviliged 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.

practicalswift · 2019-09-10T11:00:45Z

Concept ACK

maflcko · 2019-09-10T13:43:23Z

Getting rid of these globals has been talked about in past as a way
to implement testing and simulations.

Fine with me, but this seems only like a first step, since other globals such as ::mempool are still global. and not members of the Node class.

ryanofsky

Thanks for feedback!

Rebased 124bbca -> 7225ebe (pr/noglob.3 -> pr/noglob.4) due to conflict with #16787. Also added requested comment and no-wallet build fix.

re: #16839 (comment)

Fine with me, but this seems only like a first step, since other globals such as ::mempool are still global. and not members of the Node class.

Yes, it's not all or nothing. It's perfectly fine for ::mempool to remain a global, but if we want to write multiple-instance tests for code currently using ::mempool, we would have the option of moving it to the Node struct and still accessing it conveniently.

ryanofsky · 2019-09-10T15:28:41Z

src/interfaces/chain.cpp

+    {
+        ChainClients& clients = m_node.chain_clients;
+        ChainClients::iterator it = clients.emplace(clients.end(), client);
+        return MakeHandler([&clients, it] { clients.erase(it); });


Is it better to capture member chain_clients than this?

I think a good starting point is to just capture what's needed without unused data or object references. But in this case, capturing this instead of &clients is worse because in addition to capturing unneeded data, it requires the Handler object to be destroyed before the Chain object, and there's no reason to create that kind of error-prone dependency.

Also, it would be nice to explain that a std::list is used otherwise this .erase would fail.

Sure, added comment.

jamesob · 2019-09-11T17:12:41Z

Concept ACK. This looks like a step in the right direction in terms of cutting down on globals and moving towards being able to test multi-node behavior without having to rely on the functional tests.

Is your intent to split this into separate commits at some point for detailed review?

ryanofsky · 2019-09-11T17:34:50Z

Is your intent to split this into separate commits at some point for detailed review?

Could go either way. These are basically boring text replacements that should be about as much effort to review split up or joined, but I'm happy to split it up. I think I could split it up into 4 or 5 commits roughly matching the bullet points in the description. Overall diff would be a little bigger because some lines (chain_clients lines) would change more than once.

jamesob · 2019-09-11T17:38:51Z

Could go either way.

I'm fine reviewing either, though I guess if it were easy to do some scripted-diffs that would be nice.

ariard

Great concept ACK! See comments on minor points, also I think maybe the Handler/Cleanup handler logic could be simplified in future work, didn't spend time on the how yet..

ariard · 2019-09-14T14:10:34Z

src/interfaces/node.h

@@ -254,6 +255,10 @@ class Node
    using NotifyHeaderTipFn =
        std::function<void(bool initial_download, int height, int64_t block_time, double verification_progress)>;
    virtual std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0;
+
+    //! Return pointer to internal chain interface, useful for testing.
+    //! Only works if node interface is being called in same process as implementation.


I find comment fuzzy, do you mean only works if node interface is set by the process of the implementation ?

re: #16839 (comment)

I find comment fuzzy, do you mean only works if node interface is set by the process of the implementation ?

Sorry, this was pretty confusing. Better not to refer to processes in comments before #10102, so I removed this. But to answer the question: this always returns non-null currently and after #10102 will return null if the node is a remote process.

ariard · 2019-09-14T14:18:40Z

src/node/node.h

+//! Node struct containing chain state and connection state.
+//!
+//! More state could be moved into this struct to eliminate global variables,
+//! and allow creating multiple instances of validation and networking objects


So you would create multiple Node in one test process, connect them and let them talk maybe with random introduction of interferences in-between ?

re: #16839 (comment)

So you would create multiple Node in one test process, connect them and let them talk maybe with random introduction of interferences in-between ?

With caveats, yes, since code that isn't referring to global variables can be instantiated multiple times for testing in a single process.

src/node/transaction.cpp

ariard · 2019-09-14T14:25:06Z

src/rpc/blockchain.h

@@ -46,4 +47,9 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
 /** Used by getblockstats to get feerates at different percentiles by weight  */
 void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_weight);

+//! Pointer to node state that needs to be declared as a global to be accessible
+//! RPC methods. Due to limitations of the RPC framework, there's currently no


How to override RPC framework limitations ? Encapsulating RPC functions in its own structure like it has been done for the P2P stack with CConnman ?

re: #16839 (comment)

How to override RPC framework limitations ? Encapsulating RPC functions in its own structure like it has been done for the P2P stack with CConnman ?

Yes, that's one way. Another option would be to add a std::any member to the JSONRPCRequest struct, so arbitrary context could be passed into RPC methods in a type-safe way.

src/rpc/mining.cpp

-BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'struct InitInterfaces' 'struct NodeContext' s 'InitInterfaces interfaces' 'NodeContext node' s 'InitInterfaces& interfaces' 'NodeContext\& node' s 'InitInterfaces m_interfaces' 'NodeContext m_context' s 'InitInterfaces\* g_rpc_interfaces' 'NodeContext* g_rpc_node' s 'g_rpc_interfaces = &interfaces' 'g_rpc_node = \&node' s 'g_rpc_interfaces' 'g_rpc_node' s 'm_interfaces' 'm_context' s 'interfaces\.chain' 'node.chain' s '$AppInitMain\|Shutdown\|Construct$(interfaces)' '\1(node)' s 'init interfaces' 'chain clients' -END VERIFY SCRIPT-

So g_connman and g_banman globals can be removed next commit.

-BEGIN VERIFY SCRIPT- sed -i 's:#include <interfaces/chain.h>:#include <banman.h>\n#include <interfaces/chain.h>\n#include <net.h>\n#include <net_processing.h>:' src/node/context.cpp sed -i 's/namespace interfaces {/class BanMan;\nclass CConnman;\nclass PeerLogicValidation;\n&/' src/node/context.h sed -i 's/std::unique_ptr<interfaces::Chain> chain/std::unique_ptr<CConnman> connman;\n std::unique_ptr<PeerLogicValidation> peer_logic;\n std::unique_ptr<BanMan> banman;\n &/' src/node/context.h sed -i '/std::unique_ptr<[^>]\+> $g_connman\|g_banman\|peerLogic$;/d' src/banman.h src/net.h src/init.cpp sed -i 's/g_connman/m_context.connman/g' src/interfaces/node.cpp sed -i 's/g_banman/m_context.banman/g' src/interfaces/node.cpp sed -i 's/g_connman/m_node.connman/g' src/interfaces/chain.cpp src/test/setup_common.cpp sed -i 's/g_banman/m_node.banman/g' src/test/setup_common.cpp sed -i 's/g_connman/node.connman/g' src/init.cpp src/node/transaction.cpp sed -i 's/g_banman/node.banman/g' src/init.cpp sed -i 's/peerLogic/node.peer_logic/g' src/init.cpp sed -i 's/g_connman/g_rpc_node->connman/g' src/rpc/mining.cpp src/rpc/net.cpp src/rpc/rawtransaction.cpp sed -i 's/g_banman/g_rpc_node->banman/g' src/rpc/net.cpp sed -i 's/std::shared_ptr<CWallet> wallet =/node.context()->connman = std::move(test.m_node.connman);\n &/' src/qt/test/wallettests.cpp -END VERIFY SCRIPT-

Wallet code should use interfaces::Chain and not directly access to node state. Add a g_rpc_chain replacement global for wallet code to use, and move g_rpc_node definition to a libbitcoin_server source file so there are link errors if wallet code tries to access it.

maflcko · 2019-10-28T15:32:03Z

Looks ready for merge after rebase

ryanofsky · 2019-10-28T23:58:10Z

Rebased 34a7a50 -> 362ded4 (pr/noglob.15 -> pr/noglob.16) due to conflict with #16202

Also renamed struct Node to struct NodeContext, since it should also be useful to have a WalletContext later to put things like cs_wallets, vpwallets, and the interfaces::Chain pointer shared between multiple wallets.

laanwj · 2019-10-30T11:29:47Z

ACK 362ded4

362ded4 Avoid using g_rpc_node global in wallet code (Russell Yanofsky) 8922d7f scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky) e6f4f89 Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky) 4d5448c MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky) 301bd41 scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky) Pull request description: This change is mainly a naming / organization change intended to simplify #10102. It: - Renames struct InitInterfaces to struct NodeContext and moves it from src/init.h to src/node/context.h. This is a cosmetic change intended to make the point of the struct more obvious. - Gets rid of BanMan and ConnMan globals making them NodeContext members instead. Getting rid of these globals has been talked about in past as a way to implement testing and simulations. Making them NodeContext members is a way of keeping them accessible without the globals. - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This better separates node and wallet rpc methods. Node RPC methods should have access NodeContext, while wallet RPC methods should only have indirect access to node functionality via interfaces::Chain. - Adds NodeContext& references to interfaces::Chain class and the interfaces::MakeChain() function. This is needed to access ConnMan and BanMan instances without the globals. - Gets rid of redundant Node and Chain instances in Qt tests. This is needed due to the previous MakeChain change, and also makes test setup a little more straightforward. More cleanup could be done in the future, but it will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init code. ACKs for top commit: laanwj: ACK 362ded4 Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2

ariard · 2019-11-02T03:30:58Z

Post-humous ACK, diff from 50e0bb2 is renaming struct Node, integration of bip61 removal and some comments changed.

I wonder if renaming interfaces::Chain to ChainInterface would make it clearer that this class is an external interface used outside the node (by the wallet) to access chain state and not something that has to do with implementing node functionality or storing node data.

Agree may have avoid us confusion there.

Summary: -BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'struct InitInterfaces' 'struct NodeContext' s 'InitInterfaces interfaces' 'NodeContext node' s 'InitInterfaces& interfaces' 'NodeContext\& node' s 'InitInterfaces m_interfaces' 'NodeContext m_context' s 'InitInterfaces\* g_rpc_interfaces' 'NodeContext* g_rpc_node' s 'g_rpc_interfaces = &interfaces' 'g_rpc_node = \&node' s 'g_rpc_interfaces' 'g_rpc_node' s 'm_interfaces' 'm_context' s 'interfaces\.chain' 'node.chain' s '$AppInitMain\|Shutdown\|Construct$(interfaces)' '\1(node)' s 'init interfaces' 'chain clients' -END VERIFY SCRIPT- This is a parial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] : bitcoin/bitcoin@301bd41 Test Plan: ninja all check-all Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D6108

Summary: This is a partial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] : bitcoin/bitcoin@4d5448c Depends on D6108 Test Plan: ninja all check-all Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D6109

…aces Summary: So g_connman and g_banman globals can be removed next commit. bitcoin/bitcoin@e6f4f89 --- Partial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] Test Plan: ninja check-all ninja Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6409

Summary: ```-BEGIN VERIFY SCRIPT- sed -i 's:#include <interfaces/chain.h>:#include <banman.h>\n#include<interfaces/chain.h>\n#include <net.h>\n#include <net_processing.h>:' src/node/context.cpp sed -i 's/namespace interfaces {/class BanMan;\nclass CConnman;\nclass PeerLogicValidation;\n&/' src/node/context.h sed -i 's/std::unique_ptr<interfaces::Chain>chain/std::unique_ptr<CConnman> connman;\n std::unique_ptr<PeerLogicValidation> peer_logic;\nstd::unique_ptr<BanMan> banman;\n &/' src/node/context.h sed -i '/std::unique_ptr<[^>]\+> $g_connman\|g_banman\|peerLogic$;/d'src/banman.h src/net.h src/init.cpp sed -i 's/g_connman/m_context.connman/g' src/interfaces/node.cpp sed -i 's/g_banman/m_context.banman/g' src/interfaces/node.cpp sed -i 's/g_connman/m_node.connman/g' src/interfaces/chain.cpp src/test/util/setup_common.cpp sed -i 's/g_banman/m_node.banman/g' src/test/util/setup_common.cpp sed -i 's/g_connman/node.connman/g' src/init.cpp src/node/transaction.cpp sed -i 's/g_banman/node.banman/g' src/init.cpp sed -i 's/peerLogic/node.peer_logic/g' src/init.cpp sed -i 's/g_connman/g_rpc_node->connman/g' src/rpc/mining.cpp src/rpc/net.cpp src/rpc/rawtransaction.cpp sed -i 's/g_banman/g_rpc_node->banman/g' src/rpc/net.cpp sed -i 's/std::shared_ptr<CWallet> wallet =/node.context()->connman = std::move(test.m_node.connman);\n &/' src/qt/test/wallettests.cpp -END VERIFY SCRIPT-``` ALSO the changes to src/test/util/setup_common.cpp and src/rpc/blockchain.h from the following diff have been anticipated into this one because boost test failures. bitcoin/bitcoin@8922d7f --- Partial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] Test Plan: ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6418

Summary: Wallet code should use interfaces::Chain and not directly access to node state. Add a g_rpc_chain replacement global for wallet code to use, and move g_rpc_node definition to a libbitcoin_server source file so there are link errors if wallet code tries to access it. bitcoin/bitcoin@362ded4 --- Depends on D6418 Concludes backport of Core [[bitcoin/bitcoin#16839 | PR16839]] Test Plan: ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6419

Summary: -BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'struct InitInterfaces' 'struct NodeContext' s 'InitInterfaces interfaces' 'NodeContext node' s 'InitInterfaces& interfaces' 'NodeContext\& node' s 'InitInterfaces m_interfaces' 'NodeContext m_context' s 'InitInterfaces\* g_rpc_interfaces' 'NodeContext* g_rpc_node' s 'g_rpc_interfaces = &interfaces' 'g_rpc_node = \&node' s 'g_rpc_interfaces' 'g_rpc_node' s 'm_interfaces' 'm_context' s 'interfaces\.chain' 'node.chain' s '$AppInitMain\|Shutdown\|Construct$(interfaces)' '\1(node)' s 'init interfaces' 'chain clients' -END VERIFY SCRIPT- This is a parial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] : bitcoin/bitcoin@301bd41 Test Plan: ninja all check-all Differential Revision: https://reviews.bitcoinabc.org/D6108

Summary: This is a partial backport of Core [[bitcoin/bitcoin#16839 | PR16839]] : bitcoin/bitcoin@4d5448c Depends on D6108 Test Plan: - `ninja all check-all` - do an equivalent autoconf build with tests (`make check ; test/functional/test_runner.py`) Differential Revision: https://reviews.bitcoinabc.org/D6109

…ext local -BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'struct InitInterfaces' 'struct NodeContext' s 'InitInterfaces interfaces' 'NodeContext node' s 'InitInterfaces& interfaces' 'NodeContext\& node' s 'InitInterfaces m_interfaces' 'NodeContext m_context' s 'InitInterfaces\* g_rpc_interfaces' 'NodeContext* g_rpc_node' s 'g_rpc_interfaces = &interfaces' 'g_rpc_node = \&node' s 'g_rpc_interfaces' 'g_rpc_node' s 'm_interfaces' 'm_context' s 'interfaces\.chain' 'node.chain' s '$AppInitMain\|Shutdown\|Construct$(interfaces)' '\1(node)' s 'init interfaces' 'chain clients' -END VERIFY SCRIPT-

…ext local

DrahtBot added Build system GUI Mining P2P RPC/REST/ZMQ Tests Wallet labels Sep 9, 2019

fanquake added the Needs Conceptual Review label Sep 9, 2019

promag reviewed Sep 10, 2019

View reviewed changes

fanquake added Refactoring and removed Build system Mining RPC/REST/ZMQ labels Sep 10, 2019

DrahtBot added the Needs rebase label Sep 10, 2019

maflcko removed GUI P2P Tests Wallet labels Sep 10, 2019

ryanofsky force-pushed the pr/noglob branch from 124bbca to 7225ebe Compare September 10, 2019 16:08

ryanofsky commented Sep 10, 2019

View reviewed changes

DrahtBot removed the Needs rebase label Sep 10, 2019

ryanofsky force-pushed the pr/noglob branch from 7225ebe to a3dc533 Compare September 10, 2019 21:28

ariard reviewed Sep 14, 2019

View reviewed changes

ryanofsky added 5 commits October 28, 2019 10:30

MOVEONLY: Move NodeContext struct to node/context.h

4d5448c

Pass NodeContext, ConnMan, BanMan references more places

e6f4f89

So g_connman and g_banman globals can be removed next commit.

DrahtBot added the Needs rebase label Oct 28, 2019

ryanofsky force-pushed the pr/noglob branch from 34a7a50 to 362ded4 Compare October 28, 2019 18:47

ryanofsky changed the title ~~Replace Connman and BanMan globals with Node local~~ Replace Connman and BanMan globals with NodeContext local Oct 28, 2019

DrahtBot removed the Needs rebase label Oct 28, 2019

laanwj merged commit 362ded4 into bitcoin:master Oct 30, 2019

hebasto mentioned this pull request Oct 30, 2019

gui: Bilingual GUI error messages #16224

Merged

ryanofsky mentioned this pull request Nov 21, 2019

rpc: Removing g_rpc_node #17548

Closed

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

kwvg mentioned this pull request Oct 7, 2021

merge bitcoin#15931, #16839, #17192, #17407, #18037, #17997, partial #15639, #17989: deglobalisation and mining rpc backports dashpay/dash#4357

Closed

kwvg added a commit to kwvg/dash that referenced this pull request Jan 11, 2022

merge bitcoin#16839: Replace Connman and BanMan globals with NodeCont…

70afa78

…ext local

kwvg added a commit to kwvg/dash that referenced this pull request Jan 25, 2022

merge bitcoin#16839: Replace Connman and BanMan globals with NodeCont…

5bcd907

…ext local

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

Replace Connman and BanMan globals with NodeContext local #16839

Replace Connman and BanMan globals with NodeContext local #16839

ryanofsky commented Sep 9, 2019 •

edited

Loading

ryanofsky commented Sep 9, 2019

promag left a comment

promag Sep 10, 2019

ryanofsky Sep 10, 2019 •

edited

Loading

DrahtBot commented Sep 10, 2019 •

edited

Loading

practicalswift commented Sep 10, 2019

maflcko commented Sep 10, 2019

ryanofsky left a comment •

edited

Loading

ryanofsky Sep 10, 2019 •

edited

Loading

jamesob commented Sep 11, 2019

ryanofsky commented Sep 11, 2019

jamesob commented Sep 11, 2019

ariard left a comment

ariard Sep 14, 2019

ryanofsky Sep 17, 2019

ariard Sep 14, 2019

ryanofsky Sep 17, 2019

ariard Sep 14, 2019

ryanofsky Sep 17, 2019

maflcko commented Oct 28, 2019

ryanofsky commented Oct 28, 2019

laanwj commented Oct 30, 2019

ariard commented Nov 2, 2019

Replace Connman and BanMan globals with NodeContext local #16839

Replace Connman and BanMan globals with NodeContext local #16839

Conversation

ryanofsky commented Sep 9, 2019 • edited Loading

ryanofsky commented Sep 9, 2019

promag left a comment

Choose a reason for hiding this comment

promag Sep 10, 2019

Choose a reason for hiding this comment

ryanofsky Sep 10, 2019 • edited Loading

Choose a reason for hiding this comment

DrahtBot commented Sep 10, 2019 • edited Loading

Conflicts

practicalswift commented Sep 10, 2019

maflcko commented Sep 10, 2019

ryanofsky left a comment • edited Loading

Choose a reason for hiding this comment

ryanofsky Sep 10, 2019 • edited Loading

Choose a reason for hiding this comment

jamesob commented Sep 11, 2019

ryanofsky commented Sep 11, 2019

jamesob commented Sep 11, 2019

ariard left a comment

Choose a reason for hiding this comment

ariard Sep 14, 2019

Choose a reason for hiding this comment

ryanofsky Sep 17, 2019

Choose a reason for hiding this comment

ariard Sep 14, 2019

Choose a reason for hiding this comment

ryanofsky Sep 17, 2019

Choose a reason for hiding this comment

ariard Sep 14, 2019

Choose a reason for hiding this comment

ryanofsky Sep 17, 2019

Choose a reason for hiding this comment

maflcko commented Oct 28, 2019

ryanofsky commented Oct 28, 2019

laanwj commented Oct 30, 2019

ariard commented Nov 2, 2019

ryanofsky commented Sep 9, 2019 •

edited

Loading

ryanofsky Sep 10, 2019 •

edited

Loading

DrahtBot commented Sep 10, 2019 •

edited

Loading

ryanofsky left a comment •

edited

Loading

ryanofsky Sep 10, 2019 •

edited

Loading