Skip to content

test: add some unit tests for merkle.cpp #16865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
maflcko merged 1 commit into from
Sep 17, 2019
Merged

test: add some unit tests for merkle.cpp #16865

maflcko merged 1 commit into from
Sep 17, 2019

Conversation

@soroosh-sdi
Copy link
Contributor

@soroosh-sdi soroosh-sdi commented Sep 13, 2019 •
edited
Loading

following situations are covered:

  • empty block
  • one Tx
  • Merkle root of a block with odd Txs should not change with repeating
    last one
  • Merkle root is computed with combining Merkle root of left subtree and right subtree
  • block witness is Merkle root of a block when setting first Tx
    to zero.

Signed-off-by: soroosh-sdi soroosh.sardari@gmail.com

@instagibbs
Copy link
Member

instagibbs commented Sep 13, 2019

you can remove the boilerplate from "Pull requests without a rationale and clear improvement may be closed
immediately."

nice submission!

@DrahtBot DrahtBot added the Tests label Sep 13, 2019
@soroosh-sdi
Copy link
Contributor Author

soroosh-sdi commented Sep 13, 2019

Thanks bro @instagibbs

@soroosh-sdi soroosh-sdi force-pushed the 13-09-19-merkle-unit-tests branch from 2279a37 to c7b9e4d Compare September 14, 2019 02:49
theStack
theStack reviewed Sep 16, 2019
View reviewed changes
src/test/merkle_tests.cpp Outdated
mtx.nLockTime = pos;
block.vtx[pos] = MakeTransactionRef(std::move(mtx));
}
pos--;
Copy link
Contributor

@theStack theStack Sep 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can be removed, since pos is not used outside the for loop. Also, the declaration and initialization of pos can be combined.

Copy link
Contributor Author

@soroosh-sdi soroosh-sdi Sep 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I forgot to remove this, thanks for warning me.

@laanwj
Copy link
Member

laanwj commented Sep 16, 2019

Thanks for adding tests, they look good to me.

A small comment on the commit message; please format commit messages as

<title line>
<empty line>
<more description…>

Currently, due to lack of an empty line after the title, tools will smash all the description text on one line:

  • c7b9e4d test: add some unit tests for merkle.cpp following situations are covered: - empty block - one Tx - Merkle root of a block with odd Txs should not change with repeating last one - Merkle root is computed with combining Merkle root of left subtree and right subtree - block witness is Merkle root of a block when setting first Tx to zero.

instead of just

  • c7b9e4d test: add some unit tests for merkle.cpp

@soroosh-sdi soroosh-sdi force-pushed the 13-09-19-merkle-unit-tests branch from c7b9e4d to a77132a Compare September 16, 2019 18:23
@soroosh-sdi
test: add some unit tests for merkle.cpp
8573429 
following situations are covered:
- empty block
- one Tx
- Merkle root of a block with odd Txs should not change with repeating
last one
- Merkle root is computed with combining Merkle root of left subtree and right subtree
- block witness is Merkle root of a block when setting first Tx
to zero.

Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
@soroosh-sdi soroosh-sdi force-pushed the 13-09-19-merkle-unit-tests branch from a77132a to 8573429 Compare September 16, 2019 19:07
@soroosh-sdi
Copy link
Contributor Author

soroosh-sdi commented Sep 17, 2019

your welcome, commit message edited.
@laanwj

@laanwj
Copy link
Member

laanwj commented Sep 17, 2019

ACK 8573429

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Sep 17, 2019
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
b3067f4 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
@maflcko maflcko merged commit 8573429 into bitcoin:master Sep 17, 2019
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 21, 2020
@soroosh-sdi @PiRK
[backport#16865] test: add some unit tests for merkle.cpp
48baf4e 
Summary:
following situations are covered:
- empty block
- one Tx
- Merkle root of a block with odd Txs should not change with repeating
last one
- Merkle root is computed with combining Merkle root of left subtree and right subtree

Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

Backport of Core [[bitcoin/bitcoin#16865 | PR16865]]

Test Plan: `ninja && ninja check`

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8017
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
606501e 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
493e27d 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
1e3db87 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 11, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
9f47971 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 11, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
83f2f14 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 12, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
9462e30 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 12, 2021
@PastaPastaPasta
Merge bitcoin#16865: test: add some unit tests for merkle.cpp
9c94a2b 
8573429 test: add some unit tests for merkle.cpp (soroosh-sdi)

Pull request description:

  following situations are covered:
  - empty block
  - one Tx
  - Merkle root of a block with odd Txs should not change with repeating
  last one
  - Merkle root is computed with combining Merkle root of left subtree and right subtree
  - block witness is Merkle root of a block when setting first Tx
  to zero.

  Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>

ACKs for top commit:
  laanwj:
    ACK 8573429

Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
kwvg added a commit to kwvg/dash that referenced this pull request Nov 6, 2021
@kwvg
merge bitcoin#16865: add some unit tests for merkle.cpp
c69d011
kwvg added a commit to kwvg/dash that referenced this pull request Dec 5, 2021
@kwvg
merge bitcoin#16865: add some unit tests for merkle.cpp
6a775ab
kwvg added a commit to kwvg/dash that referenced this pull request Dec 5, 2021
@kwvg
merge bitcoin#16865: add some unit tests for merkle.cpp
e0b8f05
kwvg added a commit to kwvg/dash that referenced this pull request Dec 12, 2021
@kwvg
merge bitcoin#16865: add some unit tests for merkle.cpp
b0ef742
kwvg added a commit to kwvg/dash that referenced this pull request Dec 12, 2021
@kwvg
merge bitcoin#16865: add some unit tests for merkle.cpp
45e09d4
@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.

Reviewers

1 more reviewer

@theStack theStack theStack left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@soroosh-sdi @instagibbs @laanwj @theStack @maflcko @DrahtBot