[Wallet][RPC] FundTransaction - fundrawtransaction #1663

random-zebra · 2020-06-04T08:06:21Z

based on top of

[Cleanup] Remove obsolete multisig code #1662

This introduces a new wallet function, CWallet::FundTransaction() (and exposes it via RPC with fundrawtransaction), to fill a tx containing only vouts (or not enough vins to cover the vouts) with unspent coins from the wallet.

fundrawtransaction will not modify existing inputs, and will add one change output (if needed) to the outputs. It will not sign the inputs (so can include also watch-only or multi-sig inputs, if enabled).

backported from:

adapting the tests for the (more recent) framework.

[*] Note: this has been included to be able to call fundrawtransaction without the need for an unencrypted wallet (for the change address key)

random-zebra · 2020-06-09T10:15:08Z

Rebased. Ready for review/QA.

ambassador000 · 2020-06-26T01:37:05Z

fundrawtransaction RPC command is working as intended (also with encrypted wallet), however I couldn't get working createrawtransaction and signrawtransaction commands, always ended up with various errors, probably because of the confusing input format needed.

random-zebra · 2020-06-26T16:13:18Z

@NoobieDev12 , what kind of errors?
It should be straightforward.
Here's a brief tutorial to (1) create, (2) fund, (3) sign, and (4) broadcast a transaction:

Create a transaction without inputs (empty list []), only outputs (dictionary {"address": amount}):

$ ./pivx-cli createrawtransaction '[]' '{"yCrjZ7EcS2y7jFo48dMvmUxPtp1fTt9W4U": 100}'

01000000000100e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

Fund the transaction (i.e. add enough inputs to cover the output values):

$ ./pivx-cli fundrawtransaction 01000000000100e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

{
  "hex": "01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f0000000000ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000",
  "changepos": 0,
  "fee": 0.00002260
}

The raw transaction ("hex" string from the output of the previous command) is unsigned, as we can see decoding it:

$ ./pivx-cli decoderawtransaction 01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f0000000000ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

{
  "txid": "0d9ab9ea5324fc9d3d7ce98bf8b4e3bf722fc0404d105b42fd4f9b8ac7cbbcb0",
  "version": 1,
  "size": 119,
  "locktime": 0,
  "vin": [
    {
      "txid": "7fa51972658053d32b62dc8cc560be86c57ce69cab277bb00b998688c73ea54d",
      "vout": 0,
      "scriptSig": {
        "asm": "",
        "hex": ""
      },
      "sequence": 4294967295
    }
  ],
  ...
}

Let's add the signatures to the input:

$ ./pivx-cli signrawtransaction 01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f0000000000ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

{
  "hex": "01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f000000006a4730440220546266cdd03230f0c7c71aed643704651c50228c910b102c36d96780ec14c02f0220152ff071e9d67a836c7fbcc5c019e072692a836e8210830de17c5aa3501676aa012102271fb71ea03d9c9d6014d192dcda21de7d47f9044f6786b72761fb9c7034c162ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000",
  "complete": true
}

The transaction (again "hex" string from the output of the previous command) is now signed:

$ ./pivx-cli decoderawtransaction 01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f000000006a4730440220546266cdd03230f0c7c71aed643704651c50228c910b102c36d96780ec14c02f0220152ff071e9d67a836c7fbcc5c019e072692a836e8210830de17c5aa3501676aa012102271fb71ea03d9c9d6014d192dcda21de7d47f9044f6786b72761fb9c7034c162ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

{
  "txid": "6751d397bc4e97bf444248844b783b83f652a4fb900d4df85fb148b9f89ee86d",
  "version": 1,
  "size": 225,
  "locktime": 0,
  "vin": [
    {
      "txid": "7fa51972658053d32b62dc8cc560be86c57ce69cab277bb00b998688c73ea54d",
      "vout": 0,
      "scriptSig": {
        "asm": "30440220546266cdd03230f0c7c71aed643704651c50228c910b102c36d96780ec14c02f0220152ff071e9d67a836c7fbcc5c019e072692a836e8210830de17c5aa3501676aa[ALL] 02271fb71ea03d9c9d6014d192dcda21de7d47f9044f6786b72761fb9c7034c162",
        "hex": "4730440220546266cdd03230f0c7c71aed643704651c50228c910b102c36d96780ec14c02f0220152ff071e9d67a836c7fbcc5c019e072692a836e8210830de17c5aa3501676aa012102271fb71ea03d9c9d6014d192dcda21de7d47f9044f6786b72761fb9c7034c162"
      },
      "sequence": 4294967295
    }
  ],
  ...
}

Let's broadcast it to the network:

$ ./pivx-cli sendrawtransaction 01000000014da53ec78886990bb07b27ab9ce67cc586be60c58cdc622bd35380657219a57f000000006a4730440220546266cdd03230f0c7c71aed643704651c50228c910b102c36d96780ec14c02f0220152ff071e9d67a836c7fbcc5c019e072692a836e8210830de17c5aa3501676aa012102271fb71ea03d9c9d6014d192dcda21de7d47f9044f6786b72761fb9c7034c162ffffffff02ace41b78030000001976a914d764b754e913a26bce160c6a5ca2c1c20ebb8dc588ac00e40b54020000001976a914ae29d55979ebcaceccf4d660810a3390e728b5fa88ac00000000

6751d397bc4e97bf444248844b783b83f652a4fb900d4df85fb148b9f89ee86d

furszy

Code review ACK 5a41ebb.

src/wallet/wallet.cpp

src/rpc/rawtransaction.cpp

furszy

Working as intended 👌 , tested ACK 5a41ebb.

ambassador000 · 2020-06-28T00:13:15Z

Thanks for the detailed reply and explanation RZ!
All working now! However, just for the reference, I cannot reproduce it again, but yesterday at some point using the fundrawtransaction I got the "changepos": -1, output. Other errors were mostly related to invalid inputs and are irrelevant because of that.

Fuzzbawls · 2020-06-28T09:16:49Z

Needs a rebase, but otherwise ACK

- backports bitcoin/bitcoin@9b4e7d9

- backports bitcoin/bitcoin@1e0d1a2 Some code stolen from Jonas Schnelli <[email protected]>

- backports bitcoin/bitcoin@21bbd92

- backports bitcoin/bitcoin@709f868

- backports bitcoin/bitcoin@92bcd70

- backports bitcoin/bitcoin@0aad1f1 Since unspendable outputs can't be spent, there is no threshold at which it would be uneconomic to spend them. This primarily targets transaction outputs with `OP_RETURN`.

- backports bitcoin/bitcoin@d3354c5 This indicates that, eg, we have a public key for a key which may be used as a pay-to-pubkey-hash. It generally means that we can create a valid scriptSig except for missing private key(s) with which to create signatures. - backports bitcoin/bitcoin@428a898 [squash commit]

- backports bitcoin/bitcoin@cfc3dd3

- backports bitcoin/bitcoin@983d2d9

- backports bitcoin/bitcoin@a1d7df3

- backports bitcoin/bitcoin@5c17059

- backports bitcoin/bitcoin@6bdb474 Some code and test cases stolen from Bryan Bishop <[email protected]> (pull bitcoin#5524). - backports bitcoin/bitcoin@d042854 [squash commit]

- backports bitcoin/bitcoin@d6cc6a1

- backports bitcoin/bitcoin@f7dc1d3 Strict flag forces type check on all object keys.

- backports bitcoin/bitcoin@db992ea [PIVX: Don't keep backward compatibility (as this is introduced and updated in the same PR)]

- backports bitcoin/bitcoin@18be394

- backports bitcoin/bitcoin@3b35e48

- backports bitcoin/bitcoin@04eaa90

random-zebra · 2020-06-28T10:28:53Z

Rebased, and added one last case to the functional test.

furszy

re ACK fc81158 .

Fuzzbawls

ACK fc81158

d1d15c8 Fix missing sigverion in main_test.cpp CreateDummyScriptSigWithKey. (furszy) a034daf Rename to PrecomputedTransactionData (furszy) b4b181b Unit test for sighash caching (furszy) 2ef3872 Report non-mandatory script failures correctly. (furszy) 446d340 Precompute sighashes (furszy) dfd24eb Update wallet_txn_close.py test: (furszy) a5170f0 BIP143: Signing logic. (furszy) d2dd547 BIP143: Verification logic. (furszy) dccc3c6 Refactor script validation to observe amounts (furszy) daf044a Reduce unnecessary hashing in signrawtransaction (furszy) Pull request description: Base work for the new transaction digest algorithm for signature verification on PIVX Sapling transactions. Essentially, an implementation of BIP143 + few more good commits that found down the rabbit hole. Back ports: * bitcoin#7276 * bitcoin#7976 * bitcoin#8118 * bitcoin#8149 (only amount validation and SignatureHash commits). * bitcoin#6088 (only the dummy signature one - will be removed once #1663 get merged -). * bitcoin#6379 * bitcoin#8524 Next step over this area (need 1553 merged to be able to push it) is the further specialization of BIP143 into our custom implementation of ZIP143 (with a different digest algorithm definition using our tx data and hash personalization). ACKs for top commit: Fuzzbawls: utACK d1d15c8 random-zebra: ACK d1d15c8 and merging... Tree-SHA512: 7665cccf095c5bce0b18ef7ab8fcf7bede9304993b48f1af9c352c568861dec728d1d68671aab857b73d46567678492c4b97c24644a15f3f29fc4d723b183522

random-zebra added RPC Upstream Wallet Tests Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes labels Jun 4, 2020

random-zebra added this to the 4.2.0 milestone Jun 4, 2020

random-zebra self-assigned this Jun 4, 2020

random-zebra force-pushed the 202005_fundrawtransaction branch 2 times, most recently from 8771399 to ba6109b Compare June 5, 2020 20:45

furszy mentioned this pull request Jun 6, 2020

Base work for the Sapling signatureHash #1666

Merged

random-zebra force-pushed the 202005_fundrawtransaction branch from ba6109b to 5a41ebb Compare June 9, 2020 10:12

random-zebra requested review from furszy and Fuzzbawls June 19, 2020 12:20

furszy reviewed Jun 26, 2020

View reviewed changes

src/wallet/wallet.cpp Show resolved Hide resolved

furszy reviewed Jun 27, 2020

View reviewed changes

src/rpc/rawtransaction.cpp Outdated Show resolved Hide resolved

furszy previously approved these changes Jun 27, 2020

View reviewed changes

random-zebra added 9 commits June 28, 2020 12:07

Add DummySignatureCreator which just creates zeroed sigs

692b827

- backports bitcoin/bitcoin@9b4e7d9

Add FundTransaction method to wallet

ccb18dd

- backports bitcoin/bitcoin@1e0d1a2 Some code stolen from Jonas Schnelli <[email protected]>

Add fundrawtransaction RPC method

761e60e

- backports bitcoin/bitcoin@21bbd92

[wallet] CreateTransaction: simplify change address check

a2f8071

- backports bitcoin/bitcoin@709f868

[wallet] allow transaction without change if keypool is empty

bc44ba0

- backports bitcoin/bitcoin@92bcd70

[Tests] Fix and enable fundrawtransaction functional tests

ab407ff

Exempt unspendable transaction outputs from dust checks

fab6556

- backports bitcoin/bitcoin@0aad1f1 Since unspendable outputs can't be spent, there is no threshold at which it would be uneconomic to spend them. This primarily targets transaction outputs with `OP_RETURN`.

Add logic to track pubkeys as watch-only, not just scripts

cbffa80

- backports bitcoin/bitcoin@cfc3dd3

random-zebra added 15 commits June 28, 2020 12:08

Split up importaddress into helper functions

60a20a4

- backports bitcoin/bitcoin@983d2d9

Add p2sh option to importaddress to import redeemScripts

816dabb

Add importpubkey method to import a watch-only pubkey

7b4eb6d

- backports bitcoin/bitcoin@a1d7df3

Update importaddress help to push its use to script-only

1b153e5

- backports bitcoin/bitcoin@5c17059

Implement watchonly support in fundrawtransaction

134c5d2

- backports bitcoin/bitcoin@6bdb474 Some code and test cases stolen from Bryan Bishop <[email protected]> (pull bitcoin#5524). - backports bitcoin/bitcoin@d042854 [squash commit]

[QA] Test watchonly addrs in fundrawtransaction tests

76c8d54

Use CCoinControl selection in CWallet::FundTransaction

d655b42

- backports bitcoin/bitcoin@d6cc6a1

Add strict flag to RPCTypeCheckObj

0c1f7ba

- backports bitcoin/bitcoin@f7dc1d3 Strict flag forces type check on all object keys.

Add change options to fundrawtransaction

a3ac191

- backports bitcoin/bitcoin@db992ea [PIVX: Don't keep backward compatibility (as this is introduced and updated in the same PR)]

Add lockUnspents option to fundrawtransaction

bc9dc67

- backports bitcoin/bitcoin@18be394

[QA] Test new options in rpc_fundrawtransaction functional test

87dbdf8

[RPC] add feerate option to fundrawtransaction

169bc3b

- backports bitcoin/bitcoin@3b35e48

Add more clear interface for CoinControl.h regarding individual feerate

5bca4f4

- backports bitcoin/bitcoin@04eaa90

[QA] Add test_option_feerate to rpc_fundrawtransaction functional test

dd35760

[QA] Add test_change_position case to rpc_fundrawtransaction.py

fc81158

random-zebra dismissed furszy’s stale review via fc81158 June 28, 2020 10:27

random-zebra force-pushed the 202005_fundrawtransaction branch from 5a41ebb to fc81158 Compare June 28, 2020 10:27

furszy approved these changes Jun 28, 2020

View reviewed changes

Fuzzbawls approved these changes Jun 28, 2020

View reviewed changes

furszy merged commit 0724bbb into PIVX-Project:master Jun 28, 2020

furszy mentioned this pull request Jun 29, 2020

Prepare for non-Base58 addresses [Step 3] #1679

Merged

Fuzzbawls removed the Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes label Sep 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wallet][RPC] FundTransaction - fundrawtransaction #1663

[Wallet][RPC] FundTransaction - fundrawtransaction #1663

random-zebra commented Jun 4, 2020 •

edited

Loading

random-zebra commented Jun 9, 2020

ambassador000 commented Jun 26, 2020

random-zebra commented Jun 26, 2020 •

edited

Loading

furszy left a comment

furszy left a comment

ambassador000 commented Jun 28, 2020 •

edited

Loading

Fuzzbawls commented Jun 28, 2020

random-zebra commented Jun 28, 2020

furszy left a comment

Fuzzbawls left a comment

[Wallet][RPC] FundTransaction - fundrawtransaction #1663

[Wallet][RPC] FundTransaction - fundrawtransaction #1663

Conversation

random-zebra commented Jun 4, 2020 • edited Loading

random-zebra commented Jun 9, 2020

ambassador000 commented Jun 26, 2020

random-zebra commented Jun 26, 2020 • edited Loading

furszy left a comment

Choose a reason for hiding this comment

furszy left a comment

Choose a reason for hiding this comment

ambassador000 commented Jun 28, 2020 • edited Loading

Fuzzbawls commented Jun 28, 2020

random-zebra commented Jun 28, 2020

furszy left a comment

Choose a reason for hiding this comment

Fuzzbawls left a comment

Choose a reason for hiding this comment

random-zebra commented Jun 4, 2020 •

edited

Loading

random-zebra commented Jun 26, 2020 •

edited

Loading

ambassador000 commented Jun 28, 2020 •

edited

Loading