Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Sep 17, 2019

The param coins to SignTransaction is final and can thus not be extended (as suggested by the doc).

@laanwj
Copy link
Member

laanwj commented Sep 18, 2019

ACK fae8be06e7d54f70514cd5fa43423f79c2779e12

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

This was changed in #16798 39034f1 where ParsePrevouts was extracted from SignTransaction.

ACK fae8be06e7d54f70514cd5fa43423f79c2779e12.

@maflcko
Copy link
Member Author

maflcko commented Sep 19, 2019

Addressed feedback by @promag

@practicalswift
Copy link
Contributor

ACK fa8d65f -- const correctness is good and diff looks correct

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa8d65f

fanquake added a commit that referenced this pull request Sep 20, 2019
…transaction_util

fa8d65f doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util (MarcoFalke)

Pull request description:

  The param `coins` to `SignTransaction` is final and can thus not be extended (as suggested by the doc).

ACKs for top commit:
  practicalswift:
    ACK fa8d65f -- const correctness is good and diff looks correct
  fanquake:
    ACK fa8d65f

Tree-SHA512: 041e159f2c3cf96e296173c31f3e5f35bbc7711cc888aa4bf08aaa8c65c95ee7f7672f65396690a9af45795a618eea0fadde7fb02d29ec85f1b4df5e6d9e0c7a
@fanquake fanquake merged commit fa8d65f into bitcoin:master Sep 20, 2019
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 22, 2020
Summary:
The param coins to SignTransaction is final and can thus not be extended (as suggested by the doc).
This was changed in [[bitcoin/bitcoin#16798 | PR16798]] where ParsePrevouts was extracted from SignTransaction.

This is a backport of Core [[bitcoin/bitcoin#16900 | PR16900]]

Test Plan: `ninja && ninja check`

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8053
@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.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants