Suggested interfaces::Chain cleanups from #15288

ryanofsky commented at 9:31 pm on March 4, 2019: member

Mostly documentation improvements requested in the last review of #15288 before it was merged (https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-210241864)

Mostly documentation improvements requested in the last review of #15288 before
it was merged
(https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-210241864)

4d4e4c6448

fanquake added the label Refactoring on Mar 4, 2019

MarcoFalke commented at 10:32 pm on March 4, 2019: member

ACK de9f7f2d3847d6aa2ac33c3ae6e2b92227b6cebd

in src/interfaces/README.md:7 in de9f7f2d38 outdated

1@@ -2,9 +2,9 @@
2 
3 The following interfaces are defined here:
4 
5-* [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
6+* [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#14437](https://github.com/bitcoin/bitcoin/pull/14437), [#14711](https://github.com/bitcoin/bitcoin/pull/14711), [#15288](https://github.com/bitcoin/bitcoin/pull/15288), and [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
7 
8-* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
9+* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/14437).

fanquake commented at 11:02 pm on March 4, 2019:

nit: [#14437]

in src/interfaces/chain.h:31 in de9f7f2d38 outdated

27 
28 class Wallet;
29 
30-//! Interface for giving wallet processes access to blockchain state.
31+//! Interface giving clients (wallet processes, maybe other analysis tools in
32+//! the future) ability to access to chain state, receive notifications,

fanquake commented at 11:03 pm on March 4, 2019:

nit: the chain state

fanquake approved

fanquake commented at 11:05 pm on March 4, 2019: member

utACK de9f7f2

DrahtBot commented at 11:16 pm on March 4, 2019: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#10973 (Refactor: separate wallet from node by ryanofsky)
#10102 ([experimental] Multiprocess bitcoin 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.

ryanofsky force-pushed on Mar 5, 2019

ryanofsky commented at 0:24 am on March 5, 2019: member

Updated de9f7f2d3847d6aa2ac33c3ae6e2b92227b6cebd -> 4d4e4c644826db03317d69a04fea03309c3ebabf (pr/wclean2.1 -> pr/wclean2.2, compare) with suggested changes

fanquake commented at 0:35 am on March 5, 2019: member

re-utACK 4d4e4c6

in src/interfaces/chain.h:34 in 4d4e4c6448

30-//! Interface for giving wallet processes access to blockchain state.
31+//! Interface giving clients (wallet processes, maybe other analysis tools in
32+//! the future) ability to access to the chain state, receive notifications,
33+//! estimate fees, and submit transactions.
34+//!
35+//! TODO: Current chain methods are too low level, exposing too much of the

Empact commented at 11:57 am on March 5, 2019:

nit: I suspect this and below won’t be particularly helpful if included in the doxygen output

MarcoFalke commented at 1:55 pm on March 5, 2019:

I think it is helpful to describe the long term goal better (and give ideas to work on for new contributors)

ryanofsky commented at 2:00 pm on March 5, 2019:

re: #15531 (review)

nit: I suspect this and below won’t be particularly helpful if included in the doxygen output

I don’t know if this is a formatting suggestion or a content suggestion. It would help if you could be more specific. I’ve seen links posted to bitcoin doxygen output once in a while, but I’ve haven’t used the doxygen output myself. I do read comments that appear in code. And I especially like comments that tell me why a particular piece of code was designed a certain way, so I am not left in the dark about things that seem like counterintuitive decision designs.

MarcoFalke commented at 1:55 pm on March 5, 2019: member

re-utACK 4d4e4c6

MarcoFalke merged this on Mar 5, 2019

MarcoFalke closed this on Mar 5, 2019

MarcoFalke referenced this in commit d8a62db8bf on Mar 5, 2019

deadalnix referenced this in commit e6069992d7 on Apr 28, 2020

Munkybooty referenced this in commit f8345672b6 on Nov 25, 2021

Munkybooty referenced this in commit 48825dbfd3 on Nov 30, 2021

DrahtBot locked this on Dec 16, 2021

Suggested interfaces::Chain cleanups from #15288 #15531

Conflicts