Skip to content

Conversation

@jonatack
Copy link
Member

@jonatack jonatack commented Mar 24, 2020

This is the second commit of #17535.

This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in #17437. It also cleans up code in the test function without changing or removing existing checks.

@laanwj laanwj added the Tests label Mar 24, 2020
@fjahr
Copy link
Contributor

fjahr commented Mar 29, 2020

tested ACK 83e1d92

@promag
Copy link
Contributor

promag commented Mar 29, 2020

Code review 83e1d92.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 83e1d92. Nice test improvements!

re: description #18420 (comment)

This is the second commit of #17535, which adds coverage for #17437.

It's great to link to other issues, but would recommend writing PR description and that doesn't require reading other pages to understand. Would suggest starting off with something like, "This PR extends listsinceblock test to check new transaction 'blockheight' field recently added in #17437. It also cleans up code in the test function without changing or removing existing checks."

@jonatack
Copy link
Member Author

Thanks @ryanofsky for the suggestion; added it to the PR description.

@maflcko maflcko merged commit 965c0c3 into bitcoin:master Mar 30, 2020
@jonatack jonatack deleted the listsinceblock-block-height-checks branch March 30, 2020 23:24
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 31, 2020
83e1d92 test: listsinceblock block height checks (Jon Atack)

Pull request description:

  This is the second commit of bitcoin#17535.

  This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in bitcoin#17437. It also cleans up code in the test function without changing or removing existing checks.

ACKs for top commit:
  fjahr:
    tested ACK 83e1d92
  ryanofsky:
    Code review ACK 83e1d92. Nice test improvements!

Tree-SHA512: 92874b49a3bc0236500495f32dfcf683e1971ca3d4c51702c69ed4ce7dfce21273754f02f93d1243d73793701d9fdf49e14b149477cd249cbbd9e4e8d5bd49f8
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 13, 2021
Summary:
> This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in [[bitcoin/bitcoin#17437 | PR17437]]. It also cleans up code in the test function without changing or removing existing checks.

This is a backport of Core [[bitcoin/bitcoin#18420 | PR18420]]

Test Plan: `ninja && test/functional/test_runner.py wallet_listsinceblock`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D8895
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants