Skip to content

merge #13269, #13425, #13557, #13721, #13666, #13723: BIP 174 PSBT Serializations and RPCs - #4186

Merged
PastaPastaPasta merged 8 commits into
dashpay:developfrom
kwvg:psbt
Jul 13, 2021
Merged

merge #13269, #13425, #13557, #13721, #13666, #13723: BIP 174 PSBT Serializations and RPCs#4186
PastaPastaPasta merged 8 commits into
dashpay:developfrom
kwvg:psbt

Conversation

@kwvg

@kwvg kwvg commented Jun 4, 2021

Copy link
Copy Markdown
Collaborator

Overview

This pull request aims to implement BIP 174 ("Partially Signed Bitcoin Transaction Format" by Andrew Chow) without references to segregated witness (which featured heavily in the implementation), a leftover reference unimplemented Replace-by-fee (RBF) transactions and relevant changes made to CoinJoin logic that accompany the PR

Motivation

During initial preparation for bitcoin#18242 ("Add BIP324 encrypted p2p transport de-/serializer" by jonasschnelli) and the backporting of bitcoin@78c312c, a difference in class and function names were noticed and that in turn lead to a series of pull requests the lead us to this one.

The motivation is to reduce future merge conflicts when backporting pull requests from Bitcoin Core.

Contents

bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bitcoin#13666, bitcoin#13723

Disclosures

  • This is a work in progress and needs external review. Dash-specific changes have not been tested, only compilation and unit test success has been ensured (P.S.: It fails for now). Running the client on a testnet is necessary.
  • Tests fail. I do not know why, I do not know how. They fail. (see "Changes that need review")

Changes that need review

  • Anything and everything related to CoinJoin

@UdjinM6

UdjinM6 commented Jun 8, 2021

Copy link
Copy Markdown

pls see https://github.com/UdjinM6/dash/commits/pr4186 for some patches (besides 52f38ff which is a temporary hack to "fix" tests)

@PastaPastaPasta

Copy link
Copy Markdown
Member

Need to rebase to drop 0297bfb

@kwvg

kwvg commented Jul 9, 2021

Copy link
Copy Markdown
Collaborator Author

Rebased and updated, Python tests pending

@kwvg

kwvg commented Jul 13, 2021

Copy link
Copy Markdown
Collaborator Author

Python tests re-adapted for Dash

@kwvg
kwvg force-pushed the psbt branch 2 times, most recently from d7c3051 to fa1b6c0 Compare July 13, 2021 09:21

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dae890b516361c089d1181989d57a0ff602a5732 and b60872b89e7d5eddfa36ce71a9f9ee338a8bef4e should be dropped (13269 and 13425 were fully backported earlier + changes there do not match original PRs, probably some leftovers from rebase), plus see below

Comment thread test/functional/rpc_psbt.py Outdated

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, utACK

@UdjinM6
UdjinM6 requested a review from PastaPastaPasta July 13, 2021 17:54

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tests passed, looks good to me. Let's get this ****ing merged!

Comment thread src/wallet/wallet.cpp
}
std::copy(keyID.begin(), keyID.begin() + 4, info.fingerprint);
return true;
} No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit

@PastaPastaPasta
PastaPastaPasta merged commit e98241d into dashpay:develop Jul 13, 2021
@PastaPastaPasta PastaPastaPasta added this to the 18 milestone Jul 15, 2021
@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Aug 16, 2021
@kwvg
kwvg deleted the psbt branch July 18, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC Some notable changes to RPC params/behaviour/descriptions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants