Skip to content

Draft: CScript::IsPayToScriptHash Refactoring

This MR is a refactoring of the CScript::IsPayToScriptHash() function

Several functions are created, each with a single responsibility: IsPayToScriptHash20, IsPayToScriptHash32, and other helper functions.

The old IsPayToScriptHash(uint32_t) function came to have many responsibilities, and those responsibilities depended on the function arguments. This makes it more difficult to read and review, as well as test, since some of its functionality was hidden in the default parameters. In this MR it was renamed to IsPayToScriptHashDynamic, although I don't like the name, since its name doesn't indicate what the function does.

This MR introduces a potential performance improvement, as it returns a Span (similar to C++20 std::span) instead of needing a std::vector. When the user of the function needs to copy the data from the Span to a container, he can do it, but when he don't, he don't have to.

Edit: Some optimizing compilers are capable of removing all code when one part of a tuple is not used, for example:

std::tuple<A, B, C> foo() {
  return { make_a(),
           make_b(),
           make_c()};
}

auto const [a, b, _] = foo();
do_something_with(a);
do_something_with(b);
// _ is not used

Some compilers are capable to remove the make_c() invocation.

TODO

  • We need a better name and doc/summary for function IsPayToScriptHashDynamic.
  • Adds tests that cover the new functionality added in !1600 (merged).

Test Plan

  • ninja all check
Edited by Fernando Pelliccioni

Merge request reports