Skip to content

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Jun 4, 2021

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
Copy link

UdjinM6 commented Jun 8, 2021

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
Member

Need to rebase to drop 0297bfb

@kwvg
Copy link
Collaborator Author

kwvg commented Jul 9, 2021

Rebased and updated, Python tests pending

@kwvg
Copy link
Collaborator Author

kwvg commented Jul 13, 2021

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
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

dae890b and b60872b 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

Copy link

@UdjinM6 UdjinM6 left a comment

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
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

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!

}
std::copy(keyID.begin(), keyID.begin() + 4, info.fingerprint);
return true;
}
Copy link
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