Skip to content

Conversation

@domob1812
Copy link
Contributor

For constructing test scripts, use std::vector and, in particular, std::vector::insert to insert 20 zero bytes rather than listing the full array of bytes explicitly. This makes the code easier to read and makes it immediately obvious what the structure of the data is, without having to count the zeros to understand it.

Of course, that is a matter of taste - so if you disagree that the change makes the code easier to read, let me know.

This has been split out of #14752.

For constructing test scripts, use std::vector and, in particular,
std::vector::insert to insert 20 zero bytes rather than listing the full
array of bytes explicitly.  This makes the code easier to read and makes
it immediately obvious what the structure of the data is, without having
to count the zeros to understand it.
@laanwj
Copy link
Member

laanwj commented Jan 4, 2019

utACK

I don't think this is a very pressing change but OTOH I think it's marginally easier to read, and harder to make mistakes with—e.g. with a span of 0, it's easy to accidentally insert one too many or too few.

@maflcko
Copy link
Member

maflcko commented Jan 4, 2019

utACK 6b25f29

@promag
Copy link
Contributor

promag commented Jan 4, 2019

utACK 6b25f29. Does it make sense to add the following?

CScript(const std::vector<unsigned char>& v) : CScript(v.begin(), v.end()) {}

which would simplify this change (and maybe in other places).

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 4, 2019
…est data

6b25f29 Use std::vector API for construction of test data. (Daniel Kraft)

Pull request description:

  For constructing test scripts, use `std::vector` and, in particular, `std::vector::insert` to insert 20 zero bytes rather than listing the full array of bytes explicitly.  This makes the code easier to read and makes it immediately obvious what the structure of the data is, without having to count the zeros to understand it.

  Of course, that is a matter of taste - so if you disagree that the change makes the code easier to read, let me know.

  This has been split out of bitcoin#14752.

Tree-SHA512: af82d447f0077259049f1da2d6f86a6c29723c6e17bd342e9a9ecf37b13bddff40643af95c8b3a3260765a5591713d31ca8a45a5a0c20a12c139aee53ea150da
@maflcko maflcko merged commit 6b25f29 into bitcoin:master Jan 4, 2019
@domob1812 domob1812 deleted the p2sh-test-refactor branch January 5, 2019 11:53
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 2, 2020
Summary:
> For constructing test scripts, use std::vector and, in particular,
> std::vector::insert to insert 20 zero bytes rather than listing the full
> array of bytes explicitly.  This makes the code easier to read and makes
> it immediately obvious what the structure of the data is, without having
> to count the zeros to understand it.

This is a backport of Core [[bitcoin/bitcoin#15099 | PR15099]] and [[bitcoin/bitcoin#17254 | PR17254]]

[[bitcoin/bitcoin#17254 | PR17254]] adds back opcodes lost in the fist PR.

Test Plan: `ninja && ninja check`

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8236
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
…est data

6b25f29 Use std::vector API for construction of test data. (Daniel Kraft)

Pull request description:

  For constructing test scripts, use `std::vector` and, in particular, `std::vector::insert` to insert 20 zero bytes rather than listing the full array of bytes explicitly.  This makes the code easier to read and makes it immediately obvious what the structure of the data is, without having to count the zeros to understand it.

  Of course, that is a matter of taste - so if you disagree that the change makes the code easier to read, let me know.

  This has been split out of bitcoin#14752.

Tree-SHA512: af82d447f0077259049f1da2d6f86a6c29723c6e17bd342e9a9ecf37b13bddff40643af95c8b3a3260765a5591713d31ca8a45a5a0c20a12c139aee53ea150da
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants