bitcoin: implement bitcoin view #534

pkova · 2020-11-24T15:10:38Z

As per the the bounty at https://grants.urbit.org/bounties/794126751-bitcoin-key-derivation

Depends on urbit/urbit-key-generation#92.

pkova · 2020-11-24T15:12:03Z

Checking with a real bitcoin dude:

@timlucmiptev BIP-174 PSBT:s are the right tool for the job regarding unsigned transactions, right?

Edit: the answer is yes

pkova · 2020-11-24T15:13:08Z

src/views/Bitcoin.js

+
+  const [signedTx, setSignedTx] = useState();
+
+  const privKey = urbitWallet.unsafeGet().bitcoin.keys.private;


If we don't have urbitWallet here, just explode. It's checked in Point.js, wanted to give these keys as props to my component but didn't figure out how to do it with this routing setup.

pkova · 2020-11-24T19:08:19Z

Out of band todos for this one after discussing with @jalehman and @timlucmiptev:

Exposition for the bitcoin screen.
Remove unnecessary @ux dojo formatting.
Proper xPub formatting.
Sync how bitcoinjs-lib handles PSBT:s with @timlucmiptev.

galenwp · 2020-11-24T20:45:50Z

very cool

Fang- · 2020-11-30T17:15:54Z

Might be interesting to look at #522, would be nice if landscape and other clients could open directly into the relevant Bitcoin flow in Bridge, instead of requiring manual user input beyond their login details.

jalehman · 2020-11-30T17:39:52Z

Might be interesting to look at #522, would be nice if landscape and other clients could open directly into the relevant Bitcoin flow in Bridge, instead of requiring manual user input beyond their login details.

Great point @Fang- — this is an ideal use-case.

As per the the bounty at https://grants.urbit.org/bounties/794126751-bitcoin-key-derivation

pkova · 2020-12-01T22:57:13Z

Fixed all the todos listed above, the exposition text might need some work though. This is now more easily runnable since urbit/urbit-key-generation#92 is in, but generating suitable PSBT:s from the xPub is a little bit of a pain. I used https://github.com/Coldcard/psbt_faker.

Gonna look at the stuff @Fang- posted next.

jalehman · 2021-02-04T16:04:21Z

src/views/Bitcoin.js

+          {' ' + pointName}. You can paste it to Landscape to generate an
+          unsigned transaction.


I'd remove the reference to Landscape here.

From the copy here in general, it's also not clear what this does. What kind of transaction will Landscape generate? Why do I need to go somewhere else to generate that, when it looks like I have all my cryptographic information right here?

Fang-

Not sure how to test this, I can't get psbt_faker to give me anything I can paste in here to make it work.

Looking at the flow now, #522 might not be ideal for this, since you're going bridge->landscape->bridge instead of just landscape->bridge, and needing to log in twice is a bit awkward...

Fang- · 2021-02-05T12:57:57Z

src/form/Inputs.js

+  return (
+    <HexInput
+      type="text"
+      placeholder=""


Consider adding a placeholder, or simply letting this fall through to HexInput's PLACEHOLDER_HEX (though I guess that might get confusing, wrt leading 0x).

src/lib/validators.js

src/views/Bitcoin.js

src/views/Point.js

This commit allows bridge to sign a PSBT when one or more of the inputs are able to be signed by our bitcoin wallet. This allows Bridge to potentially be used in some multisig setups.

The bitcoin wallet is derived from the master ticket, not a point.

Fang- · 2021-02-24T18:12:41Z

Paging @urcades, @jyng. Still pending design review, and input on how to present access to this. It's a bit weird to display this on the individual point page, but we might also not want to open the points overview page just for this. (Currently, point overview is only shown if you control multiple points or have stars in lockup, otherwise skipping straight to the individual point page.)

Changes so far look good @pkova. There's still a couple unresolved review items from last time, but they're all small ones. I'll probably touch this soon, for flows integration, so that landscape can open Bridge into this.

Fang- · 2021-02-25T13:34:05Z

I went ahead and integrated #522 into this, in 514f31c on the m/btc-flow branch.
No idea how to get the form to process the given initialValues instead of just showing them in the form. Maybe we want an explicit "sign this" button anyway, or maybe there's a way to be clever about form behavior? cc @liam-fitzgerald

Edit: turns out this is only relevant in the error reporting case, so maybe not as important to make this fluid. Logging in with the flow set up throws you into this screen nearly instantly:

urcades · 2021-02-26T22:58:25Z

pulling in the PR now locally, will check this out and review soon

Fang- · 2021-02-26T23:35:44Z

For the record, the "try it out" instructions that worked for me:

log in as some point,
open the bitcoin view,
copy out the zpub, convert it to an xpub using this page
use psbt_faker to generate a fake tx, psbt_faker test.psbt 'xpubPasteItHere' -6
copy the contents of test.psbt into the input field in bridge

@urcades lmk if you need any bridge setup or testing help, I'll be around.

urcades · 2021-02-27T02:09:17Z

Hello, I've been unable to serve the built bridge interface, but I've been given enough background to provide some feedback on the current interface (shown above).

Right now, here's a rough sketch of the basic progression we expect when performing a bitcoin transaction:

The leftmost screen in this image displays the last stage/step of the "Landscape Crypto Invoicing" flow, which is to produce an unsigned transaction hash, which would be auto-copied to the user's clipboard (or manually copied, if auto-copying is blocked for some reason) upon clicking the "Copy Transaction and Sign in Bridge" button.

This button would open up a window to bridge.

After logging in using whatever method a pilot has associated with their Urbit ID, they should be brought immediately to the bitcoin page shown above. The copied transaction can then be copied into the "Unsigned Transaction" field.

We should add an explicit button under this field so that an urbit pilot can explicitly press a "Sign Transaction" button after pasting the transaction in the field to produce the signed transaction hash.

This modified form design can be structured as such:

Basically, I think we should make the two sections for 1) Setting Extended Pubkey in Landscape and 2) Transaction Signing very, very distinct, and make it obvious that the first section is set only once (when setting up a wallet), while the second section is used every time a transaction is made.

Also, the second signing section should really have distinct 'states' for each stage of the signing process.

Would appreciate any feedback on the above.

Also improve copywriting from design feedback, and sign transaction with explicit button.

Fang-

Alright, after talking out of band, I think this is all good now. Thanks so much for your work and patience @pkova!

Merging this into a bitcoin branch for flows integration, final touches etc.

pkova commented Nov 24, 2020

View reviewed changes

bitcoin: implement bitcoin view

fb2aecb

As per the the bounty at https://grants.urbit.org/bounties/794126751-bitcoin-key-derivation

pkova force-pushed the master branch from 32c3203 to fb2aecb Compare December 1, 2020 22:46

bitcoin: debugging stuff for demo

1926a63

pkova force-pushed the master branch from 12e7933 to 1926a63 Compare December 10, 2020 11:41

pkova added 4 commits December 29, 2020 21:53

bitcoin: use zPub instead of xPub

42d3e0b

bitcoin: sign with correct derivation path

8c35f05

bitcoin: add key prop to transaction outputs

20a0641

bitcoin: remove debugging stuff

a6b3591

jalehman requested review from jalehman and Fang- February 4, 2021 15:59

jalehman reviewed Feb 4, 2021

View reviewed changes

jalehman requested a review from urcades February 4, 2021 16:05

Fang- requested changes Feb 5, 2021

View reviewed changes

pkova added 5 commits February 7, 2021 18:04

bitcoin: remove surrounding quotes from derivation path comments

570d0a6

bitcoin: fix misnamed function xpubToYpub -> xpubToZpub

e88ec6d

bitcoin: allow partial signing

61d0d8e

This commit allows bridge to sign a PSBT when one or more of the inputs are able to be signed by our bitcoin wallet. This allows Bridge to potentially be used in some multisig setups.

bitcoin: rename hd to btcWallet

d779a1a

bitcoin: remove references to a point

c8e8454

The bitcoin wallet is derived from the master ticket, not a point.

pkova force-pushed the master branch from feddbdc to c8e8454 Compare February 7, 2021 19:33

jalehman requested a review from Fang- February 22, 2021 17:27

bitcoin: modify path in a more robust way when signing

ef3340a

urcades approved these changes Feb 27, 2021

View reviewed changes

bitcoin: split xpub and transaction signing to separate screens

fb30b41

Also improve copywriting from design feedback, and sign transaction with explicit button.

matildepark mentioned this pull request Mar 1, 2021

Bitcoin wallet and FE integration urbit/landscape#11

Closed

3 tasks

Fang- changed the base branch from master to bitcoin March 3, 2021 21:00

Fang- approved these changes Mar 3, 2021

View reviewed changes

Fang- merged commit 9916088 into urbit:bitcoin Mar 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bitcoin: implement bitcoin view #534

bitcoin: implement bitcoin view #534

pkova commented Nov 24, 2020

pkova commented Nov 24, 2020 •

edited

Loading

pkova Nov 24, 2020

pkova commented Nov 24, 2020

galenwp commented Nov 24, 2020

Fang- commented Nov 30, 2020

jalehman commented Nov 30, 2020

pkova commented Dec 1, 2020 •

edited

Loading

jalehman Feb 4, 2021

Fang- Feb 5, 2021

Fang- left a comment

Fang- Feb 5, 2021

Fang- commented Feb 24, 2021

Fang- commented Feb 25, 2021 •

edited

Loading

urcades commented Feb 26, 2021

Fang- commented Feb 26, 2021

urcades commented Feb 27, 2021 •

edited

Loading

Fang- left a comment


		const [signedTx, setSignedTx] = useState();

		const privKey = urbitWallet.unsafeGet().bitcoin.keys.private;

		{' ' + pointName}. You can paste it to Landscape to generate an
		unsigned transaction.

bitcoin: implement bitcoin view #534

bitcoin: implement bitcoin view #534

Conversation

pkova commented Nov 24, 2020

pkova commented Nov 24, 2020 • edited Loading

pkova Nov 24, 2020

Choose a reason for hiding this comment

pkova commented Nov 24, 2020

galenwp commented Nov 24, 2020

Fang- commented Nov 30, 2020

jalehman commented Nov 30, 2020

pkova commented Dec 1, 2020 • edited Loading

jalehman Feb 4, 2021

Choose a reason for hiding this comment

Fang- Feb 5, 2021

Choose a reason for hiding this comment

Fang- left a comment

Choose a reason for hiding this comment

Fang- Feb 5, 2021

Choose a reason for hiding this comment

Fang- commented Feb 24, 2021

Fang- commented Feb 25, 2021 • edited Loading

urcades commented Feb 26, 2021

Fang- commented Feb 26, 2021

urcades commented Feb 27, 2021 • edited Loading

Fang- left a comment

Choose a reason for hiding this comment

pkova commented Nov 24, 2020 •

edited

Loading

pkova commented Dec 1, 2020 •

edited

Loading

Fang- commented Feb 25, 2021 •

edited

Loading

urcades commented Feb 27, 2021 •

edited

Loading