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

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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