doc: Add external interface consistency guarantees #14592

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1810-docCon changing 3 files +45 −0
  1. MarcoFalke commented at 1:22 pm on October 28, 2018: member
    An attempt to clarify our consistency guarantees for developers and users
  2. MarcoFalke added the label Docs on Oct 28, 2018
  3. MarcoFalke commented at 1:25 pm on October 28, 2018: member
    Requested by @sdaftuar in #14193 (comment) and #14278
  4. fanquake deleted a comment on Oct 28, 2018
  5. DrahtBot commented at 3:21 pm on October 28, 2018: member
    • #14458 (WIP: Add JSON-RPC interface documentation by promag)

    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.

  6. in doc/JSON-RPC-interface.md:17 in faed8d54e4 outdated 
    12+However, the state of modules that depend on the mempool may not be up-to-date
13+with the current mempool state.
14+
15+In practice that means:
16+
17+* The mempool state returned via an RPC is consistent with itself and with the
    ryanofsky commented at 1:52 pm on October 29, 2018:

    Should this say “mempool state returned via non-wallet RPCs” instead of “mempool state returned via an RPCs” since “wallet may not be up-to-date with the current state of the mempool”?

    This might be clearer if it started off saying there are different types of RPCs (wallet and non-wallet RPCs), and then talked about each type of RPC in its own paragraph.

    MarcoFalke commented at 2:57 pm on October 29, 2018:

    I wouldn’t call it different types of RPCs, but rather each rpc is part of one module. Modules could be:

    • net
    • chainstate
    • mempool
    • wallet
    • …
  7. in doc/JSON-RPC-interface.md:21 in faed8d54e4 outdated 
    16+
17+* The mempool state returned via an RPC is consistent with itself and with the
18+  chainstate at the time of the call. Thus, the mempool state only encompasses
19+  transactions that are considered mine-able by the node at the time of the
20+  RPC. The mempool state may not reflect all effects of blocks and transactions
21+  that were sent on the P2P interface, but it reflects all effects of mempool
    ryanofsky commented at 1:56 pm on October 29, 2018:
    Can you give an example? Is this saying there that after a P2P response showing a certain state, there could be an RPC response returning an earlier state? Or is this just talking about incoming P2P messages that haven’t been fully processed yet?
    MarcoFalke commented at 2:58 pm on October 29, 2018:

    Only talking about the processing delay/buffer.

    Maybe I should just remove this sentence?

  8. in doc/JSON-RPC-interface.md:24 in faed8d54e4 outdated 
    19+  transactions that are considered mine-able by the node at the time of the
20+  RPC. The mempool state may not reflect all effects of blocks and transactions
21+  that were sent on the P2P interface, but it reflects all effects of mempool
22+  and chainstate related RPCs that returned prior to this call.
23+* The wallet state returned via an RPC is consistent with itself and with the
24+  chainstate at the time of the call. The effects of all blocks (and
    ryanofsky commented at 2:07 pm on October 29, 2018:

    Would add a paragraph break before “The effects of all blocks…” because this is now changing topic to wallet RPCs.

    Also maybe begin the paragraph with a sentence like “Wallet RPCs will return the latest chain state consistent with prior non-wallet RPCs, but not necessarily the latest mempool state” to give the next sentences some context.

  9. in doc/JSON-RPC-interface.md:10 in faed8d54e4 outdated 
     5+`-server` option. In the GUI it is possible to execute RPC methods in the Debug
 6+Console Dialog.
 7+
 8+## RPC consistency guarantees
 9+
10+State of modules that can be queried via RPCs is guaranteed to be at least
    ryanofsky commented at 2:13 pm on October 29, 2018:
    What is a module? Could this be changed to say what a module is, or give examples or modules, or just avoid the term?
    MarcoFalke commented at 3:12 pm on October 29, 2018:
    Removed the mention of “modules”
  10. in doc/JSON-RPC-interface.md:11 in faed8d54e4 outdated 
     6+Console Dialog.
 7+
 8+## RPC consistency guarantees
 9+
10+State of modules that can be queried via RPCs is guaranteed to be at least
11+up-to-date with the chainstate immediately prior to the call's execution.
    ryanofsky commented at 2:15 pm on October 29, 2018:
    Could this say “chain state” instead of “chainstate” to sound less jargonny and be clear this isn’t referring to the chainstate folder?
  11. in doc/JSON-RPC-interface.md:12 in faed8d54e4 outdated 
     7+
 8+## RPC consistency guarantees
 9+
10+State of modules that can be queried via RPCs is guaranteed to be at least
11+up-to-date with the chainstate immediately prior to the call's execution.
12+However, the state of modules that depend on the mempool may not be up-to-date
    ryanofsky commented at 2:19 pm on October 29, 2018:
    Is this just talking about the wallet? Or are there other modules? This paragraph is vague and confusing to me, and I think it might be clearer if it just talked about RPC methods instead of modules.
    MarcoFalke commented at 3:11 pm on October 29, 2018:
    Removed the mention of “modules”
  12. ryanofsky approved
  13. ryanofsky commented at 2:26 pm on October 29, 2018: member
    utACK faed8d54e4ab10406417e4149aa0620e18b2308b. It’s good to add this documentation. I think it could be improved some more and I left some suggestions, but it’s already helpful in its current form.
  14. MarcoFalke force-pushed on Oct 29, 2018
  15. MarcoFalke force-pushed on Oct 29, 2018
  16. in doc/JSON-RPC-interface.md:23 in 47bb44fb7c outdated 
    18+chain state at the time of the call. Thus, the mempool state only encompasses
19+transactions that are considered mine-able by the node at the time of the
20+RPC.
21+
22+The mempool state may not reflect all effects of blocks and transactions
23+that were sent on the P2P interface, but it reflects all effects of mempool
    ryanofsky commented at 4:19 pm on October 29, 2018:
    sent by this node, or sent by other nodes?
  17. in doc/JSON-RPC-interface.md:22 in 47bb44fb7c outdated 
    17+The mempool state returned via an RPC is consistent with itself and with the
18+chain state at the time of the call. Thus, the mempool state only encompasses
19+transactions that are considered mine-able by the node at the time of the
20+RPC.
21+
22+The mempool state may not reflect all effects of blocks and transactions
    ryanofsky commented at 4:28 pm on October 29, 2018:
    Maybe change “mempool state” with “mempool state returned via an RPC” to be consistent with paragraph above, and keep this grounded in RPCs, instead of the mempool abstractly.
  18. in doc/JSON-RPC-interface.md:35 in 47bb44fb7c outdated 
    30+
31+Wallet RPCs will return the latest chain state consistent with prior
32+non-wallet RPCs, but not necessarily the latest mempool state. The effects of
33+all blocks (and transactions in blocks) at the time of the call is reflected
34+in the state of all wallet transactions. For example, if a block contains
35+transactions that conflicted with mempool transactions, the wallet would
    ryanofsky commented at 4:36 pm on October 29, 2018:
    Maybe I’m misunderstanding, but I think it’s confusing to keep mentioning the mempool in this paragraph, when mempool consistency isn’t explained until the next paragraph. Maybe drop “but not necessarily the latest mempool state” in the first sentence, and replace “mempool transactions” with “unconfirmed transactions” in this sentence.
  19. ryanofsky approved
  20. ryanofsky commented at 4:41 pm on October 29, 2018: member
    utACK 47bb44fb7c43ddf293c5398ad0d65303eda135ca
  21. in doc/JSON-RPC-interface.md:12 in cb97f99f6a outdated 
     7+
 8+## RPC consistency guarantees
 9+
10+State that can be queried via RPCs is guaranteed to be at least up-to-date with
11+the chain state immediately prior to the call's execution. However, the state
12+returned by RPCs that depend on the mempool may not be up-to-date with the
    ryanofsky commented at 6:44 pm on October 30, 2018:
    Maybe say “reflect the mempool” rather than “depend on the mempool” to be more concrete.
  22. ryanofsky approved
  23. ryanofsky commented at 6:55 pm on October 30, 2018: member

    utACK cb97f99f6a38711af8a0a1e90400cd36774dfc3f. This looks good, and since it’s documentation-only, maybe it could be merged, though it would be nice to have someone else take a look.

    I keep suggesting new things every time I read this, so definitely feel free to ignore my suggestions.

  24. doc: Add external interface consistency guarantees fa77aaa5ad
  25. MarcoFalke force-pushed on Oct 30, 2018
  26. laanwj commented at 3:17 pm on November 1, 2018: member

    utACK fa77aaa5ad21563dd18ce3e407d391d37ac8c201

    appveyor failure seems BS

  27. laanwj merged this on Nov 1, 2018
  28. laanwj closed this on Nov 1, 2018
  29. laanwj referenced this in commit f69d92299d on Nov 1, 2018
  30. MarcoFalke deleted the branch on Nov 1, 2018
  31. jasonbcox referenced this in commit 494656788b on Oct 6, 2020
  32. PastaPastaPasta referenced this in commit 20ceeeb877 on Jun 27, 2021
  33. PastaPastaPasta referenced this in commit c7ca22e258 on Jun 28, 2021
  34. PastaPastaPasta referenced this in commit 23d235bc2f on Jun 29, 2021
  35. PastaPastaPasta referenced this in commit 25a4702f2e on Jul 1, 2021
  36. PastaPastaPasta referenced this in commit f83c5b0b96 on Jul 1, 2021
  37. PastaPastaPasta referenced this in commit 840006b0f6 on Jul 1, 2021
  38. PastaPastaPasta referenced this in commit 8d4a65d196 on Jul 3, 2021
  39. MarcoFalke locked this on Sep 8, 2021


MarcoFalke DrahtBot ryanofsky laanwj

Labels
Docs

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-18 21:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me