btc: Expose sign_message() to TS API and create UI #54
Conversation
asi345
force-pushed
the
btc_sign_message
branch
2 times, most recently
from
62b4661
to
9ae3378
Compare
src/wasm/mod.rs
Outdated
| pub async fn btc_sign_message( | ||
| &self, | ||
| coin: types::TsBtcCoin, | ||
| script_config: Option<types::TsBtcScriptConfigWithKeypath>, |
There was a problem hiding this comment.
pls make this not an Option<>, it is requried after all. I missed this in the previous review - you'll also need to change it in btc.rs.
There was a problem hiding this comment.
Okay, changing it in the Rust side too.
src/wasm/types.rs
Outdated
| type BtcSignMessageSignature = { | ||
| sig: Uint8Array, | ||
| recid: bigint, | ||
| electrum_sig65: Uint8Array, |
There was a problem hiding this comment.
In JS/TS, camelCase is more common, and we use it in this project too. electrumSig65.
You can use #[serde(rename_all = "camelCase")] on the SignMessageSignature struct to see if that works (check other instances or the serde docs).
src/wasm/types.rs
Outdated
| #[derive(serde::Serialize)] | ||
| pub struct BtcSignMessageSignature { | ||
| pub sig: Vec<u8>, | ||
| pub recid: u8, | ||
| pub electrum_sig65: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
no need to replicate it here, right? the struct already exists in btc::SignMessageSignature.
There was a problem hiding this comment.
You're right, I replicated it since ETH had a similar struct there, but it may not be necessary
There was a problem hiding this comment.
When I remove this, I get into errors and compiler can not find btc::SignMessageSignature, so I kept it
There was a problem hiding this comment.
Fixed it, the error was due to the serialization definition for the struct
sandbox/src/Bitcoin.tsx
Outdated
| setResult(undefined); | ||
| setErr(undefined); | ||
| try { | ||
| const signature = await bb02.btcSignMessage(coin, script_config_w_keypath, stringToUint8Array(message)); |
There was a problem hiding this comment.
you are using the hardcoded config - use keypath and simpleType to construct the one made from the user input
There was a problem hiding this comment.
Sorry, my mistake, I forgot it while testing
sandbox/src/Bitcoin.tsx
Outdated
| const [result, setResult] = useState<bitbox.BtcSignMessageSignature | undefined>(); | ||
| const [err, setErr] = useState<bitbox.Error>(); | ||
|
|
||
| const simpleTypeOptions: bitbox.BtcSimpleType[] = ['p2wpkhP2sh', 'p2wpkh', 'p2tr']; |
There was a problem hiding this comment.
just remove p2tr here as it is not supported anyway?
sandbox/src/Bitcoin.tsx
Outdated
| @@ -232,6 +232,90 @@ function BtcSignPSBT({ bb02 }: Props) { | |||
| ); | |||
| } | |||
|
|
|||
| function BtcSignMessage({ bb02 }: Props) { | |||
| const [coin, setCoin] = useState<bitbox.BtcCoin>('btc'); | |||
There was a problem hiding this comment.
remove this state, sign msg only works for btc anyway.
There was a problem hiding this comment.
Alright, coin is hardcoded to btc now
sandbox/src/Bitcoin.tsx
Outdated
| <label> | ||
| Simple Type | ||
| <select value={simpleType} onChange={(e: ChangeEvent<HTMLSelectElement>) => setSimpleType(e.target.value as bitbox.BtcSimpleType)}> | ||
| {simpleTypeOptions.map(option => <option key={option} value={option} disabled={option === 'p2tr' && (coin === 'ltc' || coin === 'tltc')}>{option}</option>)} |
There was a problem hiding this comment.
no need for these checks, just remove them from the input options. only btc for p2wsh and p2wsh-p2sh is supported by the firmware.
There was a problem hiding this comment.
Yes, no need for them after removing p2tr
sandbox/src/Bitcoin.tsx
Outdated
| <label> | ||
| Keypath | ||
| </label> | ||
| <textarea value={keypath} onChange={e => setKeypath(e.target.value)} rows={1} cols={80} /> |
There was a problem hiding this comment.
<input type="string" ...> is the normal way for a single line input. in this textarea field, if you hit enter, you can enter more lines, which is not needed for the keypath.
There was a problem hiding this comment.
I had set the rows to 1 and thought it would serve the same purpose, but after reading this I saw it was not the same. Adjusted it to the input tag.
sandbox/src/Bitcoin.tsx
Outdated
| rows={32} | ||
| readOnly | ||
| defaultValue={parsedResult} |
asi345
force-pushed
the
btc_sign_message
branch
2 times, most recently
from
21be20c
to
de376e3
Compare
src/wasm/mod.rs
Outdated
| .await?; | ||
|
|
||
| Ok( | ||
| serde_wasm_bindgen::to_value(&crate::btc::SignMessageSignature { |
There was a problem hiding this comment.
I think you can just do serde_wasm_bindgen::to_value(signature) right? no need to create a copy of the struct instance.
There was a problem hiding this comment.
Now that I use the actual struct from btc, yes I can I suppose.
sandbox/src/Bitcoin.tsx
Outdated
| const coin = 'btc'; | ||
| const simpleTypeOptions: bitbox.BtcSimpleType[] = ['p2wpkhP2sh', 'p2wpkh']; | ||
|
|
||
| const script_config: bitbox.BtcScriptConfig = { simpleType }; |
There was a problem hiding this comment.
use camelCase in Javascript, scriptConfig (also below)
|
Perfect! According to the last small details, pushing one last version. |
asi345
force-pushed
the
btc_sign_message
branch
from
de376e3
to
0d81c12
Compare
sandbox/src/Bitcoin.tsx
Outdated
| const simpleTypeOptions: bitbox.BtcSimpleType[] = ['p2wpkhP2sh', 'p2wpkh']; | ||
|
|
||
| const scriptConfig: bitbox.BtcScriptConfig = { simpleType }; | ||
| const scriptConfigWKeypath: bitbox.BtcScriptConfigWithKeypath = { scriptConfig: scriptConfig, keypath: keypath }; |
There was a problem hiding this comment.
scriptConfig: scriptConfig => scriptConfig now after the rename 😁
There was a problem hiding this comment.
True, using the shorter notation in both of the keys then.
After implementing BTC message signing for Rust API, further extensions are added to make wasm expose this functionality to TypeScript API. Finally, an extra field added under the Bitcoin section of sandbox UI app for BTC message signing. Signed-off-by: asi345 <[email protected]>
asi345
force-pushed
the
btc_sign_message
branch
from
0d81c12
to
a0845b9
Compare
After implementing BTC message signing for Rust API, further extensions are added to make wasm expose this functionality to TypeScript API. Finally, an extra field added under the Bitcoin section of sandbox UI app for BTC message signing.