Skip to content

Conversation

@jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Nov 22, 2019

The code used to produce a message hash when signing and verifying
messages was repeated in a few locations. Additionally, the "magic" text
used to prefix the message before hashing is unrelated to validation.
Create a utility function for this code rather than spreading it across
multiple files.

For prior discussion, see #17399 (comment).

The topmost three commits are for this PR. The remaining are from #17399, which this is based on. Please review that before this PR.

This removes boilerplate code in the subclasses which otherwise only
differ by the result type.
The code used to produce a message hash when signing and verifying
messages was repeated in a few locations. Additionally, the "magic" text
used to prefix the message before hashing is unrelated to validation.
Create a utility function for this code rather than spreading it across
multiple files.
The purpose of using a preamble or "magic" text as part of signing and
verifying a message was not given when the code was repeated in a few
locations. Make a test showing how it is used to prevent inadvertently
signing a transaction.
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 22, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #17624 (net: Fix an uninitialized read in ProcessMessage(…, "tx", …) when receiving a transaction we already have by practicalswift)
  • #17577 (refactor: deduplicate the message sign/verify code by vasild)
  • #17485 (net processing: Don't reach into CBlockIndex to check for block mutation by jnewbery)
  • #17479 (Return BlockValidationState from ProcessNewBlock if CheckBlock/AcceptBlock fails by jnewbery)
  • #16702 (p2p: supplying and using asmap to improve IP bucketing in addrman by naumenkogs)
  • #16688 (log: Add validation interface logging by jkczyz)
  • #16658 (validation: Rename CheckInputs to CheckInputScripts by jnewbery)
  • #15606 ([experimental] UTXO snapshots by jamesob)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 28, 2020

Closing as this has been taken care of by #17577.

@jkczyz jkczyz closed this Feb 28, 2020
@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants