Page MenuHomePhabricator

tracing: drop block_connected hash.toString() arg
ClosedPublic

Authored by Fabien on Nov 28 2022, 11:13.

Details

Reviewers
sdulfari
Group Reviewers
Restricted Project
Commits
rABC8e9ae4ef68c5: tracing: drop block_connected hash.toString() arg
Summary
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).

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

See also: https://github.com/bitcoin/bitcoin/pull/22902#discussion_r705176691

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

Backport of core#23302.

Also updated sigops => sigchecks in the bt script, and updated the detection so it's more obvious that it's enabled.

Test Plan

Build and run with tracing enabled, then:

sudo bpftrace ../contrib/tracing/connectblock_benchmark.bt 0 0 25

Event Timeline

Finally managed to get the test plan to work, by building from source and linking against my system libraries.

Fabien published this revision for review.Nov 30 2022, 15:17
Fabien edited the summary of this revision. (Show Details)
sdulfari added a subscriber: sdulfari.
sdulfari added inline comments.
contrib/tracing/connectblock_benchmark.bt
123 ↗(On Diff #36823)

Use of sigchk isn't greppable when looking for sigcheck. Is there a reason it is abbreviated?

133 ↗(On Diff #36823)

ditto

This revision is now accepted and ready to land.Nov 30 2022, 17:08
contrib/tracing/connectblock_benchmark.bt
123 ↗(On Diff #36823)

yes the reason is that the string is over the 65 char limit otherwise (note also I didn't add a trailing 's' for the same reason)

133 ↗(On Diff #36823)

actually it is this one that causes issues, the above is for consistency