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.

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

Add fundrawtransaction #288

Merged
merged 28 commits into from
Jan 16, 2018
Merged

Add fundrawtransaction #288

merged 28 commits into from
Jan 16, 2018

Conversation

dagurval
Copy link
Member

Adds fundrawtransaction RPC call. Includes the following from Bitcoin Core:
bitcoin/bitcoin#6088 - fundrawtransaction
bitcoin/bitcoin#6417 - [QA] fix possible reorg issue in (fund)rawtransaction(s).py RPC test
bitcoin/bitcoin#6444 - Exempt unspendable transaction outputs from dust checks
bitcoin/bitcoin#6415 - Implement watchonly support in fundrawtransaction
bitcoin/bitcoin#6828 - [rpc-tests] fundrawtransaction: Update fee after minRelayTxFee increase
commit bebe58 from bitcoin/bitcoin#7296
bitcoin/bitcoin#7506 - Use CCoinControl selection in CWallet::FundTransaction
bitcoin/bitcoin#7518 - Add multiple options to fundrawtransaction
bitcoin/bitcoin#7967 - [RPC] add feerate option to fundrawtransaction

Includes the updates up until their test framework supported python3. There are more changes that should be ported over after this.

@dgenr8
Copy link
Member

dgenr8 commented Dec 27, 2017

Awesome! With this we can support tools like atomic swaps decred/atomicswap#37

@dgenr8
Copy link
Member

dgenr8 commented Jan 16, 2018

Can 86ce0cc be removed?

This doesn't actually seem to be included (and doesn't look like it's needed in xt since we don't manipulate the min relay fee):
bitcoin/bitcoin#6828 - [rpc-tests] fundrawtransaction: Update fee after minRelayTxFee increase
commit bebe58 from bitcoin/bitcoin#7296

sipa and others added 28 commits January 16, 2018 10:08
Some code stolen from Jonas Schnelli <jonas.schnelli@include7.ch>
….py RPC test

- added missing mempool sync between block generations
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`.

---

Initially based on:

commit 9cf0ae26350033d43d5dd3c95054c0d1b1641eda
Author: zathras-crypto <zathrasc@gmail.com>
Date:   Wed Mar 25 02:04:02 2015 -0700

Changes:

- cherry-picked on top of bitcoin:master
- added RPC test for fundrawtransaction
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.
Some code and test cases stolen from
Bryan Bishop <bryan@ledgerx.com> (pull #5524).
Note from cherry-picker: Skipped changes in mempool_limit
Includes following commits from Bitcoin Core:
777799 - [qa] Fix pyton syntax in rpc tests
fab389 - [qa] rpc-test: Normalize assert()
fa524d - [qa] Use python2/3 syntax
fa2cea - [qa] rpc-tests: Properly use integers, floats
faa41e - [qa] py2: Unfiddle strings into bytes explicitly
Strict flag forces type check on all object keys.
Includes following commits from Bitcoin Core:
fa389d - [qa] Switch to py3
fac934 - [qa] Remove hardcoded "4 nodes" from test_framework
- Mark methods as override, as not to shadow parent.
- Remove default nHashType value, so not to risk hashtype being
interpreted as amount.
- Use SIGHASH_FORKID
@dagurval
Copy link
Member Author

  • Removed 86ce0cc
  • Rebased
  • Fixed a couple of signed/unsigned comparison warnings

@dgenr8 # 6828 is included as commit fef887a, bebe58 from # 7296 is included as commit 40812c6

@dgenr8
Copy link
Member

dgenr8 commented Jan 16, 2018

Ok, the file was collapsed when I happened to search for that code.

@dgenr8 dgenr8 self-requested a review January 16, 2018 21:44
@dgenr8 dgenr8 merged commit 59bd031 into bitcoinxt:master Jan 16, 2018
@dagurval dagurval deleted the fundrawtx branch January 17, 2018 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants