Merge bitcoin/bitcoin#23302: tracing: drop GetHash().ToString() argument from the validation:block_connected tracepoint

53c9fa9 tracing: drop block_connected hash.toString() arg (0xb10c)

Pull request description:

  The tracepoint `validation:block_connected` was introduced in #22006.
  The first argument was the hash of the connected block as a pointer
  to a C-like String. The last argument passed the hash of the
  connected block as a pointer to 32 bytes. The hash was only passed as
  string to allow `bpftrace` scripts to print the hash. It was
  (incorrectly) assumed that `bpftrace` cannot hex-format and print the
  block hash given only the hash as bytes.

  The block hash can be printed in `bpftrace` by calling
  `printf("%02x")` for each byte of the hash in an `unroll () {...}`.
  By starting from the last byte of the hash, it can be printed in
  big-endian (the block-explorer format).

  ```C
    $p = $hash + 31;
    unroll(32) {
        $b = *(uint8*)$p;
        printf("%02x", $b);
        $p -= 1;
    }
  ```

  See also: #22902 (comment)

  This is a breaking change to the block_connected tracepoint API, however
  this tracepoint has not yet been included in a release.

ACKs for top commit:
  laanwj:
    Concept and code review ACK 53c9fa9
  jb55:
    ACK 53c9fa9

Tree-SHA512: f1b9e4e0ee45aae892e8bf38e04b5ee5fbc643d6e7e27d011b829ed8701dacf966a99b7c877c46cca8666b894a375633e62582c552c8203614c6f2b9c4087585

Author: fanquake

contrib/tracing/README.md

@@ -176,17 +176,12 @@ third acts as a duration threshold in milliseconds. When the `ConnectBlock()`
 function takes longer than the threshold, information about the block, is
 printed. For more details, see the header comment in the script.
 
-By default, `bpftrace` limits strings to 64 bytes due to the limited stack size
-in the kernel VM. Block hashes as zero-terminated hex strings are 65 bytes which
-exceed the string limit. The string size limit can be set to 65 bytes with the
-environment variable `BPFTRACE_STRLEN`.
-
 The following command can be used to benchmark, for example, `ConnectBlock()`
 between height 20000 and 38000 on SigNet while logging all blocks that take
 longer than 25ms to connect.
 
 ```
-$ BPFTRACE_STRLEN=65 bpftrace contrib/tracing/connectblock_benchmark.bt 20000 38000 25
+$ bpftrace contrib/tracing/connectblock_benchmark.bt 20000 38000 25
 ```
 
 In a different terminal, starting Bitcoin Core in SigNet mode and with

contrib/tracing/connectblock_benchmark.bt

@@ -4,11 +4,8 @@
 
   USAGE:
 
-  BPFTRACE_STRLEN=65 bpftrace contrib/tracing/connectblock_benchmark.bt <start height> <end height> <logging threshold in ms>
+  bpftrace contrib/tracing/connectblock_benchmark.bt <start height> <end height> <logging threshold in ms>
 
-  - The environment variable BPFTRACE_STRLEN needs to be set to 65 chars as
-    strings are limited to 64 chars by default. Hex strings with Bitcoin block
-    hashes are 64 hex chars + 1 null-termination char.
   - <start height> sets the height at which the benchmark should start. Setting
     the start height to 0 starts the benchmark immediately, even before the
     first block is connected.
@@ -23,15 +20,15 @@
 
   EXAMPLES:
 
-  BPFTRACE_STRLEN=65 bpftrace contrib/tracing/connectblock_benchmark.bt 300000 680000 1000
+  bpftrace contrib/tracing/connectblock_benchmark.bt 300000 680000 1000
 
   When run together 'bitcoind -reindex', this benchmarks the time it takes to
   connect the blocks between height 300.000 and 680.000 (inclusive) and prints
   details about all blocks that take longer than 1000ms to connect. Prints a
   histogram with block connection times when the benchmark is finished.
 
 
-  BPFTRACE_STRLEN=65 bpftrace contrib/tracing/connectblock_benchmark.bt 0 0 500
+  bpftrace contrib/tracing/connectblock_benchmark.bt 0 0 500
 
   When running together 'bitcoind', all newly connected blocks that
   take longer than 500ms to connect are logged. A histogram with block
@@ -107,14 +104,23 @@ usdt:./src/bitcoind:validation:block_connected /arg1 >= $1 && (arg1 <= $2 || $2
 */
 usdt:./src/bitcoind:validation:block_connected / (uint64) arg5 / 1000> $3 /
 {
-  $hash_str = str(arg0);
+  $hash = arg0;
   $height = (int32) arg1;
   $transactions = (uint64) arg2;
   $inputs = (int32) arg3;
   $sigops = (int64) arg4;
   $duration = (int64) arg5;
 
-  printf("Block %d (%s)  %4d tx  %5d ins  %5d sigops  took %4d ms\n", $height, $hash_str, $transactions, $inputs, $sigops, (uint64) $duration / 1000);
+
+  printf("Block %d (", $height);
+  /* Prints each byte of the block hash as hex in big-endian (the block-explorer format) */
+  $p = $hash + 31;
+  unroll(32) {
+      $b = *(uint8*)$p;
+      printf("%02x", $b);
+      $p -= 1;
+  }
+  printf(")  %4d tx  %5d ins  %5d sigops  took %4d ms\n", $transactions, $inputs, $sigops, (uint64) $duration / 1000);
 }
 
 

doc/tracing.md

@@ -101,19 +101,12 @@ Is called *after* a block is connected to the chain. Can, for example, be used
 to benchmark block connections together with `-reindex`.
 
 Arguments passed:
-1. Block Header Hash as `pointer to C-style String` (64 characters)
+1. Block Header Hash as `pointer to unsigned chars` (i.e. 32 bytes in little-endian)
 2. Block Height as `int32`
 3. Transactions in the Block as `uint64`
 4. Inputs spend in the Block as `int32`
 5. SigOps in the Block (excluding coinbase SigOps) `uint64`
 6. Time it took to connect the Block in microseconds (µs) as `uint64`
-7. Block Header Hash as `pointer to unsigned chars` (i.e. 32 bytes in little-endian)
-
-Note: The 7th argument can't be accessed by bpftrace and is purposefully chosen
-to be the block header hash as bytes. See [bpftrace argument limit] for more
-details.
-
-[bpftrace argument limit]: #bpftrace-argument-limit
 
 ## Adding tracepoints to Bitcoin Core
 

src/validation.cpp

@@ -1877,14 +1877,13 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
     int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4;
     LogPrint(BCLog::BENCH, "    - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5 - nTime4), nTimeIndex * MICRO, nTimeIndex * MILLI / nBlocksTotal);
 
-    TRACE7(validation, block_connected,
-        block.GetHash().ToString().c_str(),
+    TRACE6(validation, block_connected,
+        block.GetHash().data(),
         pindex->nHeight,
         block.vtx.size(),
         nInputs,
         nSigOpsCost,
-        GetTimeMicros() - nTimeStart, // in microseconds (µs)
-        block.GetHash().data()
+        GetTimeMicros() - nTimeStart // in microseconds (µs)
     );
 
     return true;