doc: Use precise permission flags where possible #19474

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2007-docPerm changing 10 files +37 −45
  1. MarcoFalke commented at 3:50 PM on July 9, 2020: member

    Instead of mentioning the all-encompassing -whitelist* settings, change the docs to mention the exact permission flag that will influence the behaviour.

    This is needed because in the future, the too-broad -whitelist* settings (they either include all permission flags or apply to all peers) might be deprecated to require the permission flags to be enumerated.

    Alternatively, in the future there could be an RPC to set the net permission flags on an existing connection, in which case the -whitelist* terminology is of no help.

  2. MarcoFalke added the label Docs on Jul 9, 2020
  3. jonatack approved
  4. jonatack commented at 4:10 PM on July 9, 2020: contributor

    ACK fa52c2c

  5. laanwj commented at 5:21 PM on July 9, 2020: member

    ACK fa52c2c90fec1971a83fae08fe5fcad0be9b8833

  6. Sjors commented at 6:07 PM on July 9, 2020: member

    ACK fa52c2c

    Maybe also change these log messages:

    • Not relaying non-mempool transaction %s from whitelisted peer
    • Force relaying tx %s from whitelisted peer
    • Warning: not punishing whitelisted peer
    • Timeout downloading headers from whitelisted peer=%d, not disconnecting

    And clarify code comment:

    • // Reset in-flight state in case of whitelist
  7. MarcoFalke commented at 6:31 PM on July 9, 2020: member

    // Reset in-flight state in case of whitelist

    I have no idea what that comment means

  8. jnewbery commented at 6:37 PM on July 9, 2020: member

    Concept ACK

  9. Sjors commented at 6:44 PM on July 9, 2020: member

    // Reset in-flight state in case of whitelist

    I have no idea what that comment means

    Isn't that totally obvious from its introduction by @TheBlueMatt in #8068? :-)

    Found a comment:

    We should clear the block as no longer being in flight. Otherwise we're relying on the Misbehaving() to cause a disconnect, which would eventually clear the state -- but this seems error prone, eg if the node happens to be whitelisted.

    So I think it's OK for "whitelisted" to be vague here.

  10. DrahtBot commented at 2:26 AM on July 10, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19306 (refactor: Replace RecursiveMutex with Mutex in CTxMemPool by hebasto)
    • #18044 (Use wtxid for transaction relay by sdaftuar)
    • #15937 (Add loadwallet and createwallet load_on_startup options by ryanofsky)
    • #15936 (Unify bitcoin-qt and bitcoind persistent settings by ryanofsky)
    • #15935 (Add <datadir>/settings.json persistent settings storage by ryanofsky)
    • #15454 (Remove the automatic creation and loading of the default wallet by achow101)

    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.

  11. MarcoFalke force-pushed on Jul 10, 2020
  12. in src/rpc/blockchain.cpp:801 in fa09d3e566 outdated 
     798 | @@ -799,7 +799,7 @@ static CBlock GetBlockChecked(const CBlockIndex* pblockindex)
 799 |      if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) {
 800 |          // Block not found on disk. This could be because we have the block
 801 |          // header in our index but don't have the block (for example if a
    jnewbery commented at 10:15 AM on July 10, 2020:

    I think we can just remove everything in the parens here. Presumably if a peer sends us headers and the users requests that block over RPC before we've downloaded it we'll hit this condition too?

    MarcoFalke commented at 10:32 AM on July 10, 2020:

    thanks. Shortened the comment a bit. Also force pushed a typo fix and fixed a test failure.

  13. jnewbery commented at 10:17 AM on July 10, 2020: member

    ACK fa09d3e5665bf404b7bd388dd881e04d08df13a9.

    One comment. Feel free to ignore.

  14. MarcoFalke force-pushed on Jul 10, 2020
  15. DrahtBot cross-referenced this on Jul 10, 2020 from issue Add loadwallet and createwallet load_on_startup options by ryanofsky
  16. DrahtBot cross-referenced this on Jul 10, 2020 from issue interfaces: Expose settings.json methods to GUI by ryanofsky
  17. DrahtBot cross-referenced this on Jul 10, 2020 from issue Add <datadir>/settings.json persistent settings storage by ryanofsky
  18. DrahtBot cross-referenced this on Jul 10, 2020 from issue Remove the automatic creation and loading of the default wallet by achow101
  19. jnewbery commented at 12:13 PM on July 10, 2020: member

    Code review ACK fa6ec1403bc9cb6b967b1fc380eb8e5d3aed0c0e

  20. jnewbery cross-referenced this on Jul 10, 2020 from issue [net processing] Reduce cs_main scope in MaybeDiscourageAndDisconnect() by jnewbery
  21. doc: Use precise permission flags where possible fab5586122
  22. in test/functional/p2p_permissions.py:111 in fa6ec1403b outdated 
     107 | @@ -108,9 +108,9 @@ def check_tx_relay(self):
 108 |          block_op_true = self.nodes[0].getblock(self.nodes[0].generatetoaddress(100, ADDRESS_BCRT1_P2WSH_OP_TRUE)[0])
 109 |          self.sync_all()
 110 |  
 111 | -        self.log.debug("Create a connection from a whitelisted wallet that rebroadcasts raw txs")
 112 | +        self.log.debug("Create a connection from a forecerelay peer that rebroadcasts raw txs")
    jonatack commented at 12:33 PM on July 10, 2020:

    forcerelay

    MarcoFalke commented at 1:49 PM on July 10, 2020:

    Thanks, fixed typo

  23. MarcoFalke force-pushed on Jul 10, 2020
  24. jnewbery commented at 1:54 PM on July 10, 2020: member

    Did you intentionally revert net_processing.cpp L3575?

  25. MarcoFalke commented at 2:17 PM on July 10, 2020: member

    Yes, because it is part of https://github.com/bitcoin/bitcoin/pull/19472/files#diff-eff7adeaec73a769788bb78858815c91R3588 now

    No need to make the same changes in two different prs and conflict each other.

  26. jnewbery commented at 2:27 PM on July 10, 2020: member

    reACK fab558612278909df93bdf88f5727b04f13aef0f

    Yes, because it is part of https://github.com/bitcoin/bitcoin/pull/19472/files#diff-eff7adeaec73a769788bb78858815c91R3588 now

    Oh! I just made that change because I assumed that this PR would be merged first and I'd have to rebase anyway.

  27. DrahtBot cross-referenced this on Jul 10, 2020 from issue refactor: Replace RecursiveMutex with Mutex in CTxMemPool by hebasto
  28. fjahr commented at 8:08 PM on July 10, 2020: contributor

    Code review ACK fab558612278909df93bdf88f5727b04f13aef0f

  29. MarcoFalke closed this on Jul 10, 2020
  30. MarcoFalke reopened this on Jul 10, 2020
  31. DrahtBot cross-referenced this on Jul 11, 2020 from issue Use wtxid for transaction relay by sdaftuar
  32. jonatack commented at 7:12 AM on July 11, 2020: contributor

    ACK fab558612278909df93bdf88f5727b04f13aef0f

  33. MarcoFalke merged this on Jul 11, 2020
  34. MarcoFalke closed this on Jul 11, 2020
  35. MarcoFalke deleted the branch on Jul 11, 2020
  36. MarcoFalke cross-referenced this on Jul 15, 2020 from issue Cache responses to GETADDR to prevent topology leaks by naumenkogs
  37. Fabcien referenced this in commit b5e61384aa on Aug 30, 2021
  38. bitcoin locked this on Feb 15, 2022
Contributors
MarcoFalkejonatacklaanwjSjorsjnewberyDrahtBotfjahr
Labels
Docs
Linked (view graph)
#8068 Compact Blocks#18991 Cache responses to GETADDR to prevent topology leaks#19472 [net processing] Reduce cs_main scope in MaybeDiscourageAndDisconnect()
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: 2026-05-19 06:53 UTC