Skip to content

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Nov 1, 2018

This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).

Tidies up the way we import deterministic addresses, requested in review comment here: #14468 (comment).

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.

utACK ef91935ad045b60017924c990983bc0474f26ec0. I only superficially understand this change, but it seems to do what's described and simplify things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe needs comment. Why override if just calling super?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops. vestigial from a previous version. Removed.

This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).
@jnewbery jnewbery force-pushed the deprecate_generate2 branch from ef91935 to 3fd7e76 Compare November 1, 2018 16:53
@fanquake fanquake added the Tests label Nov 1, 2018
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 1, 2018

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

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.

utACK 3fd7e76, only change since last review is removing unneeded override.

@maflcko
Copy link
Member

maflcko commented Nov 2, 2018

utACK 3fd7e76

@maflcko maflcko merged commit 3fd7e76 into bitcoin:master Nov 2, 2018
maflcko pushed a commit that referenced this pull request Nov 2, 2018
3fd7e76 [tests] Move deterministic address import to setup_nodes (John Newbery)

Pull request description:

  This requires a small changes to a few tests, but means that
  deterministic addresses will always be imported (unless setup_nodes
  behaviour is explicitly overridden).

  Tidies up the way we import deterministic addresses, requested in review comment here: #14468 (comment).

Tree-SHA512: 2b32edf500e286c463398487ab1153116a1dc90f64a53614716373311abdc83d8a251fdd8f42d1146b56e308664deaf62952113f66e98bc37f23968096d1a961
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 6, 2020
Summary:
Adds a generate() method to the TestNode class in the test framework.
This method intercepts calls to generate, imports a dewterministic
private key to the node and then calls generatetoaddress to generate the
block to that address.

Note that repeated calls to importprivkey for the same private keys are
no-ops, so it's fine to call the generate() method many times.

This is a partial backport of Core [[bitcoin/bitcoin#14468 | PR14468]] : bitcoin/bitcoin@aab8172

The code was slightly tweaked because we have some tests for which `self.rpc` is `None` because they've been created after [[bitcoin/bitcoin#14631 | PR14631]] which remove the call to importprivkey

Depends on D5969

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D5970
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 6, 2020
Summary:
This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).

This is a backport of Core [[bitcoin/bitcoin#14631 | PR14631]]

Depends on D5972

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5973
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

5 participants