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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23545 (scripted-diff: Use clang-tidy syntax for C++ named arguments by MarcoFalke)

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.

Concept ACK

Any specific reason for closing?

@laanwj No, that was just a mistake as I was deleting old branches and this one was deleted by mistake too. Sorry.

@kiminuo No problem! Will take a look it seems very close to ready to merge.

Code review and lightly tested ACK 08d129a
Small nit on the documentation but feel free to ignore.

src/bitcoin-cli help getblock doesn't return anything related to "verbosity = 3" on
macOS Big Sur. Did I do something wrong in the steps below?

git clone https://github.com/kiminuo/bitcoin.git 23320
cd 23320
git checkout feature/2021-10-20-verbosity-level-3-nits      
./autogen.sh
./configure --with-incompatible-bdb --without-miniupnpc --without-natpmp --disable-bench --disable-wallet --without-gui
make -j 3 src/bitcoin-cli
./src/bitcoin-cli help getblock

src/bitcoin-cli help getblock doesn't return anything related to "verbosity = 3" on macOS Big Sur. Did I do something wrong in the steps below?

@pg156: You have to build and run bitcoind, as the help texts to the RPC calls are residing in there. bitcoin-cli is merely the RPC client that fetches it, i.e. most likely have another instance running on your system that has nothing to do with this PR. For simple tests like those, I recommend running bitcoind with -nolisten -noconnect to prevent unnecessary network activity.

Thank you @theStack! I get "verbosity = 3" after building bitcoind and stopping another bitcoind running on my machine, as you suggested.

@pg156 It's certainly better to update it. Updated. Thank you.

Ideas about better wordings are welcome :)

Code review ACK 3a6a670.
(but 3a6a670 doesn't exist in the "Commits" tab after the force-push. I don't know if the standard practice is to ACK only what is in the tab?)

• Tested the difference in output of getblock between verbosity of 2 and 3 is as described in RPC help, by adding two lines of logging in _test_getblock function in test/functional/rpc_blockchain.py:

self.log.info(node.getblock(blockhash, 2))
self.log.info(node.getblock(blockhash, 3))

The difference is:

'prevout': {'generated': True, 'height': 18, 'value': Decimal('50.00000000'), 'scriptPubKey': {'asm': '0 4ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260', 'hex': '00204ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260', 'address': 'bcrt1qft5p2uhsdcdc3l2ua4ap5qqfg4pjaqlp250x7us7a8qqhrxrxfsqseac85', 'type': 'witness_v0_scripthash'}}

• Tested "verbosity = 3" section in RPC help is as expected by running:

./src/bitcoind -testnet -daemon         
./src/bitcoin-cli -testnet help getblock

which generates:

...
Result (for verbosity = 3):
{                                        (json object)
  ...,                                   Same output as verbosity = 2
  "tx" : [                               (json array)
    {                                    (json object)
      "vin" : [                          (json array)
        {                                (json object)
          ...,                           The same output as verbosity = 2
          "prevout" : {                  (json object) (Only if undo information is available)
            "generated" : true|false,    (boolean) Coinbase or not
            "height" : n,                (numeric) The height of the prevout
            "value" : n,                 (numeric) The value in BTC
            "scriptPubKey" : {           (json object)
              "asm" : "str",             (string) The asm
              "hex" : "str",             (string) The hex
              "address" : "str",         (string, optional) The Bitcoin address (only if a well-defined address exists)
              "type" : "str"             (string) The type, eg 'pubkeyhash'
            }
          }
        },
        ...
      ]
    },
    ...
  ]
}

(but 3a6a670 doesn't exist in the "Commits" tab after the force-push. I don't know if the standard practice is to ACK only what is in the tab?)

Typically one ACKs the last commit from https://github.com/bitcoin/bitcoin/pull/23320/commits page because that's the current version of the PR.

You can also possibly ACK a commit from previous git (force) push to say something like "I really like the previous changeset as opposed to the current changeset" but that would be rare AFAIK.

Thank you for the review!

@pg156 Could you please re-review?
@MarcoFalke Would you mind having a look, please?

ACK 059f88b

re-ACK 059f88b