Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remote staking with P2WSH #592

Merged
merged 18 commits into from
Feb 26, 2019
Merged

Remote staking with P2WSH #592

merged 18 commits into from
Feb 26, 2019

Conversation

Nizametdinov
Copy link
Member

@Nizametdinov Nizametdinov commented Feb 12, 2019 •
edited
Loading

This PR implements witness v2 programs which allow staking of P2WSH outputs. See UIP-15.

@Nizametdinov Nizametdinov requested a review from a team February 12, 2019 17:00
cmihai
cmihai reviewed Feb 12, 2019
View reviewed changes
@cmihai cmihai requested a review from a team February 13, 2019 13:12
@Nizametdinov Nizametdinov force-pushed the remote-staking-p2wsh branch 2 times, most recently from e10efd9 to 12ae4d1 Compare February 14, 2019 17:36
cmihai
cmihai approved these changes Feb 20, 2019
View reviewed changes
Copy link
Member

@cmihai cmihai left a comment

Choose a reason for hiding this comment

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

utACK

scravy
scravy reviewed Feb 22, 2019
View reviewed changes
@Nizametdinov Nizametdinov requested a review from scravy February 25, 2019 10:33
scravy
scravy approved these changes Feb 25, 2019
View reviewed changes
Copy link
Member

@scravy scravy left a comment

Choose a reason for hiding this comment

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

utACK 6f6d3b9

@Ruteri
Copy link
Member

Ruteri commented Feb 25, 2019

Has to be rebased, as it will break p2p_segwit.py after #678. See change https://github.com/dtr-org/unit-e/pull/678/files#diff-7a31c514dd642ccd6d9940d4096109efR1095. The fix should be similar to what has already been done in multiple tests with increasing the invalid witness program value.

@Nizametdinov
Add TX_WITNESS_V2_REMOTE_STAKING_SCRIPTHASH
9003d3c 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Signing and verifying TX_WITNESS_V2_REMOTE_STAKING_SCRIPTHASH
6d15a1d 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix test
6eb4d68 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
IsMine and IsStakeableByMe for TX_WITNESS_V2_REMOTE_STAKING_SCRIPTHASH
c6bdcf9 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix feature_segwit
61a4e36 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Shorter names for remote staking txnouttypes
3ecfc2d 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Take into account remote staking scripthash outputs in GetRemoteStaki…
65148eb 
…ngBalance

Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Test sending to remote staking scripthash outputs
d87900e 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix code format
b0b1b79 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
TX_PAYVOTESLASH cannot be nested in P2(W)SH
78223ff 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix feature_remote_staking for Python 3.4
74928f7 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Rename WitnessProgram::IsRemoteStaking to IsRemoteStakingP2PKH
8f7edda 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix available coins test
e464c1d 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Correctly count number of sigops for RSP2SH
b7cb72c 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Add RSP2WSH output to AvailableCoins wallet test
d02af43 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Use empty() instead of size() == 0
4db2ec2 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Add W to IsRemoteStakingP2SH and IsRemoteStakingP2PKH
d32a48b 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
@Nizametdinov
Fix p2p_segwit test
625f017 
Signed-off-by: Azat Nizametdinov <azat@thirdhash.com>
Ruteri
Ruteri approved these changes Feb 26, 2019
View reviewed changes
Copy link
Member

@Ruteri Ruteri left a comment

Choose a reason for hiding this comment

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

utACK 625f017

@Nizametdinov Nizametdinov merged commit 1fa60fd into dtr-org:master Feb 26, 2019
@Nizametdinov Nizametdinov mentioned this pull request Feb 26, 2019
Closed
kostyantyn
kostyantyn reviewed Feb 26, 2019
View reviewed changes
src/script/ismine.cpp
@@ -264,7 +280,8 @@ bool IsStakeableByMe(const CKeyStore &keystore, const CScript &script_pub_key)
{
case TX_PUBKEYHASH:
case TX_WITNESS_V0_KEYHASH:
case TX_WITNESS_V1_REMOTE_STAKING: {
case TX_WITNESS_V1_RS_KEYHASH:
case TX_WITNESS_V2_RS_SCRIPTHASH: {
Copy link
Member

@kostyantyn kostyantyn Feb 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Would prefer to have the full name REMOTESTAKE instead of RS for clarity. I don't think there is an issue that the name is too long. And splitting the if condition by 2 lines is also fine.

kostyantyn
kostyantyn reviewed Feb 26, 2019
View reviewed changes
src/script/standard.h
@@ -64,7 +64,8 @@ enum txnouttype
TX_NULL_DATA, //!< unspendable OP_RETURN script that carries data
TX_WITNESS_V0_SCRIPTHASH,
TX_WITNESS_V0_KEYHASH,
TX_WITNESS_V1_REMOTE_STAKING,
TX_WITNESS_V1_RS_KEYHASH, //!< UIP-15 Remote staking script with public key hash spending destination
TX_WITNESS_V2_RS_SCRIPTHASH, //!< UIP-25 Remote staking script with script hash spending destination
Copy link
Member

Choose a reason for hiding this comment

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

We don't have UIP-25. We decided to extend UIP-15

@kostyantyn kostyantyn mentioned this pull request Feb 26, 2019
Merged
@Nizametdinov Nizametdinov deleted the remote-staking-p2wsh branch February 27, 2019 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostyantyn kostyantyn kostyantyn left review comments

@frolosofsky frolosofsky frolosofsky left review comments

@cmihai cmihai cmihai approved these changes

@scravy scravy scravy approved these changes

@Ruteri Ruteri Ruteri approved these changes

@castarco castarco Awaiting requested review from castarco

Assignees
No one assigned
Labels
consensus wallet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Nizametdinov @Ruteri @cmihai @castarco @scravy @kostyantyn @frolosofsky