Add new bitcoin@0.17.0 RPC methods to sign raw transactions #80

lautarodragan · 2018-11-13T06:52:04Z

Part of #77

Not sure this is right. I just copy-pasted the current signRawTransaction, changing the version and name, and updated the version for the deprecated signRawTransaction.

Tested the "happy path" case — didn't test that the version filters are working, only that I could correctly call this method. That much works well.

src/methods.js

lautarodragan · 2018-12-03T19:56:02Z

Thank you for your review @pedrobranco!

I went ahead and made the requested changes on signRawTransactionWithWallet.

I also added signRawTransactionWithKey as requested, but I'm not sure whether the obfuscate field is right. (Basically copied what is in place for the deprecated signRawTransaction).

The docs for signRawTransactionWithKey state:

Sign inputs for raw transaction (serialized, hex-encoded).
The second argument is an array of base58-encoded private
keys that will be the only keys used to sign the transaction.
The third optional argument (may be null) is an array of previous transaction outputs that
this transaction depends on but may not yet be in the block chain.

src/methods.js

pedrobranco

Squash all commits, and use a commit message like Add new bitcoin@0.17 RPC methods to sign raw transactions.

pedrobranco · 2018-12-06T15:13:14Z

Thank you for your review @pedrobranco!

Anytime.

I also added signRawTransactionWithKey as requested, but I'm not sure whether the obfuscate field is right. (Basically copied what is in place for the deprecated signRawTransaction).

The old one was the third argument, in this new method is the second argument. I've already pushed a comment here.

BTW, there is another PR for adding these methods #84. I think we can merge your PR first (despite this PR being a subset of the #84 ).

And many thanks for the pull request 👍

lautarodragan · 2018-12-11T00:15:06Z

@pedrobranco all done I think! Awaiting your input regarding the upper bound in the version range.

src/methods.js

pedrobranco · 2018-12-11T11:58:17Z

@pedrobranco all done I think! Awaiting your input regarding the upper bound in the version range.

Done.

lautarodragan · 2018-12-12T13:06:35Z

@pedrobranco added the multi wallet feature and removed the version cap. I need to squash again, but other than that, is it looking good?

pedrobranco · 2018-12-12T15:24:11Z

@pedrobranco added the multi wallet feature and removed the version cap. I need to squash again, but other than that, is it looking good?

Just add a test for the new multi-wallet method just to be sure that it works fine. To help I've bumped the docker image for the multi-wallet daemon to 0.17.

pedrobranco · 2019-02-08T11:16:00Z

@lautarodragan Any news on the missing test? We would like to add this to the new release.

lautarodragan · 2019-03-13T06:03:43Z

@pedrobranco sorry for the delay! just pushed a commit with mutli wallet tests.

jeamick · 2019-04-01T16:16:48Z

Hey @pedrobranco & @lautarodragan,

Any news for this PR ? do we just need to change the commit message ?
Thanks

Kind regards
JM

@pedrobranco @lautarodragan

lautarodragan · 2019-04-04T05:16:20Z

@jeamick @pedrobranco I'll squash the commits but I'd rather wait for feedback first.

I'd like to suggest enabling squash-merging for this repository. It allows a clean master without losing details / history in PRs, and simplifies communication between maintainers and contributors.

jeamick · 2019-04-04T08:03:42Z

I did the change and compiled it into my repository and it works for signRawTransactionWithKey.
here is the link : https://github.com/jeamick/bitcoin-core

jeamick · 2019-05-23T12:31:52Z

Any update on this PR ? @lautarodragan @pedrobranco

lautarodragan · 2019-05-30T01:37:25Z

@jeamick I think we can consider this repo pretty much abandoned. I decided to fork it and publish here.

Haven't had a chance to try it yet, if you do please let me know.

I'll be making several changes to the project soon. I intend to keep it compatible with new bitcoin-core releases and add TypeScript type definitions (which I already have for some calls in a different project), as well as fixing some limitations we faced at Po.et.

ruimarinho · 2019-05-30T01:57:57Z

This repo is definitely not abandoned @lautarodragan, but we have been focused on other projects at the moment. This library remains tried and tested. Looking forward to bring back my attention to it.

Which limitations are you trying to solve? Are those public?

lautarodragan · 2019-05-30T03:01:38Z

@ruimarinho that's great news! I think the biggest issue is how we're lagging behind Bitcoin Core releases.

Adding TypeScript definitions and making it compatible with modern ES6 modules would be nice too (although you are setting exports.default = Client so maybe it was an issue on my side).

Another issue I found, albeit minor, was trying to use the methods "outside" the object. I don't know how to explain this properly, but for example doing const { createRawTransaction } = bitcoinCore; createRawTransation(...) failed, if my memory serves me right. Had the same issue trying to pass the functions to Ramda's pipe. I think .bind(bitcoinCore) solved it, but it'd be nice not to need that binding, but it's not very idiomatic.

jeamick · 2019-05-30T18:58:17Z

@jeamick I think we can consider this repo pretty much abandoned. I decided to fork it and publish here.

Haven't had a chance to try it yet, if you do please let me know.

I'll be making several changes to the project soon. I intend to keep it compatible with new bitcoin-core releases and add TypeScript type definitions (which I already have for some calls in a different project), as well as fixing some limitations we faced at Po.et.

I already did the changes on my repo and it is working : https://github.com/jeamick/bitcoin-core

lautarodragan · 2019-05-30T19:27:31Z

Awesome! Thanks @jeamick. Please let me know if you give the published version a try.

jeamick · 2019-05-30T19:42:24Z

Awesome! Thanks @jeamick. Please let me know if you give the published version a try.

Which published version ?

I am actually using it for a personal project involving RPC request to bitcoin-core node and everything is working as expected

lautarodragan · 2019-05-31T06:12:42Z

Awesome! Thanks @jeamick. Please let me know if you give the published version a try.

Which published version ?

I am actually using it for a personal project involving RPC request to bitcoin-core node and everything is working as expected

This one: https://www.npmjs.com/package/bitcoins/v/1.0.1

pedrobranco

Fix the minor nits and it's good to merge. Sorry about the delay 😞

pedrobranco · 2019-06-26T16:53:04Z

test/multi_wallet_test.js

@@ -182,6 +168,28 @@ describe('Multi Wallet', () => {
        });
      });
    });
+
+    describe('signRawTransactionWithWallet()', () => {
+      const getFundedTransaction = async () => {


To keep consistency, please remove this function and define inside each test.

pedrobranco · 2019-06-26T16:53:35Z

test/multi_wallet_test.js

+      it('should sign a funded raw transaction and return the hex', async () => {
+        const fundedTransaction = await getFundedTransaction();
+        const signedTransaction = await client.signRawTransactionWithWallet(fundedTransaction.hex);
+        signedTransaction.should.have.keys('hex');


Newline here missing (between await calls and asserts).

pedrobranco · 2019-06-26T16:53:53Z

test/multi_wallet_test.js

+      it('should support named parameters', async () => {
+        const fundedTransaction = await getFundedTransaction();
+        const signedTransaction = await client.signRawTransactionWithWallet({ hexstring: fundedTransaction.hex });
+        signedTransaction.should.have.keys('hex');


Newline here missing (between await calls and asserts).

pedrobranco · 2019-06-27T10:54:58Z

I will submit the fix for the tests and merge this. Also, I've squashed all commits.

pedrobranco

LGTM.

lautarodragan mentioned this pull request Nov 13, 2018

chore(bitcoin-core): use signrawtransactionwithwallet poetapp/node#762

Closed

pedrobranco requested changes Nov 28, 2018

View reviewed changes

src/methods.js Show resolved Hide resolved

src/methods.js Outdated Show resolved Hide resolved

src/methods.js Outdated Show resolved Hide resolved

pedrobranco requested changes Dec 6, 2018

View reviewed changes

src/methods.js Outdated Show resolved Hide resolved

src/methods.js Outdated Show resolved Hide resolved

src/methods.js Outdated Show resolved Hide resolved

pedrobranco changed the title ~~Add signRawTransactionWithWallet~~ Add new bitcoin@0.17.0 RPC methods to sign raw transactions Dec 6, 2018

pedrobranco requested changes Dec 6, 2018

View reviewed changes

lautarodragan force-pushed the patch-1 branch from 49b2d89 to 5744267 Compare December 11, 2018 00:13

pedrobranco reviewed Dec 11, 2018

View reviewed changes

src/methods.js Show resolved Hide resolved

jeamick mentioned this pull request May 23, 2019

RPC changes on Bitcoin v0.18.0 #90

Closed

pedrobranco requested changes Jun 26, 2019

View reviewed changes

Add new bitcoin@0.17 RPC methods to sign raw transactions

eebb7df

pedrobranco force-pushed the patch-1 branch from a2def9b to eebb7df Compare June 27, 2019 10:58

pedrobranco approved these changes Jun 27, 2019

View reviewed changes

pedrobranco merged commit 60a7a5c into ruimarinho:master Jun 27, 2019

lautarodragan deleted the patch-1 branch June 27, 2019 21:35

snyk-bot mentioned this pull request Mar 7, 2020

[Snyk] Upgrade bitcoin-core from 2.0.0 to 2.3.0 AndreiShybin/btc-rpc-explorer#7

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new bitcoin@0.17.0 RPC methods to sign raw transactions #80

Add new bitcoin@0.17.0 RPC methods to sign raw transactions #80

lautarodragan commented Nov 13, 2018

lautarodragan commented Dec 3, 2018 •

edited

Loading

pedrobranco left a comment

pedrobranco commented Dec 6, 2018

lautarodragan commented Dec 11, 2018

pedrobranco commented Dec 11, 2018

lautarodragan commented Dec 12, 2018

pedrobranco commented Dec 12, 2018

pedrobranco commented Feb 8, 2019

lautarodragan commented Mar 13, 2019

jeamick commented Apr 1, 2019 •

edited

Loading

lautarodragan commented Apr 4, 2019

jeamick commented Apr 4, 2019 •

edited

Loading

jeamick commented May 23, 2019 •

edited

Loading

lautarodragan commented May 30, 2019 •

edited

Loading

ruimarinho commented May 30, 2019

lautarodragan commented May 30, 2019

jeamick commented May 30, 2019

lautarodragan commented May 30, 2019

jeamick commented May 30, 2019

lautarodragan commented May 31, 2019

pedrobranco left a comment •

edited

Loading

pedrobranco Jun 26, 2019

pedrobranco Jun 26, 2019

pedrobranco Jun 26, 2019

pedrobranco commented Jun 27, 2019 •

edited

Loading

pedrobranco left a comment

Add new bitcoin@0.17.0 RPC methods to sign raw transactions #80

Add new bitcoin@0.17.0 RPC methods to sign raw transactions #80

Conversation

lautarodragan commented Nov 13, 2018

lautarodragan commented Dec 3, 2018 • edited Loading

pedrobranco left a comment

Choose a reason for hiding this comment

pedrobranco commented Dec 6, 2018

lautarodragan commented Dec 11, 2018

pedrobranco commented Dec 11, 2018

lautarodragan commented Dec 12, 2018

pedrobranco commented Dec 12, 2018

pedrobranco commented Feb 8, 2019

lautarodragan commented Mar 13, 2019

jeamick commented Apr 1, 2019 • edited Loading

lautarodragan commented Apr 4, 2019

jeamick commented Apr 4, 2019 • edited Loading

jeamick commented May 23, 2019 • edited Loading

lautarodragan commented May 30, 2019 • edited Loading

ruimarinho commented May 30, 2019

lautarodragan commented May 30, 2019

jeamick commented May 30, 2019

lautarodragan commented May 30, 2019

jeamick commented May 30, 2019

lautarodragan commented May 31, 2019

pedrobranco left a comment • edited Loading

Choose a reason for hiding this comment

pedrobranco Jun 26, 2019

Choose a reason for hiding this comment

pedrobranco Jun 26, 2019

Choose a reason for hiding this comment

pedrobranco Jun 26, 2019

Choose a reason for hiding this comment

pedrobranco commented Jun 27, 2019 • edited Loading

pedrobranco left a comment

Choose a reason for hiding this comment

lautarodragan commented Dec 3, 2018 •

edited

Loading

jeamick commented Apr 1, 2019 •

edited

Loading

jeamick commented Apr 4, 2019 •

edited

Loading

jeamick commented May 23, 2019 •

edited

Loading

lautarodragan commented May 30, 2019 •

edited

Loading

pedrobranco left a comment •

edited

Loading

pedrobranco commented Jun 27, 2019 •

edited

Loading