Skip to content

Commit 8f5c10a

Browse files
benmabenma
committed
firmware/btc: return struct in BTCSign and BTCSignMessage
Better clarity and composes better with generic functions returning a single value.
1 parent 0bc64fb commit 8f5c10a

File tree

5 files changed

+73
-49
lines changed

5 files changed

+73
-49
lines changed

‎api/firmware/btc.go

+56-32
Original file line numberDiff line numberDiff line change
@@ -277,22 +277,29 @@ type BTCTx struct {
277277
PaymentRequests []*messages.BTCPaymentRequestRequest
278278
}
279279

280+
// BTCSignResult is the result of `BTCSign()`.
281+
type BTCSignResult struct {
282+
// Signatures contains the input signatures. One 64 byte signature per input.
283+
Signatures [][]byte
284+
// GeneratedOutputs contains the outputs generated (silent payments). The map key is the input
285+
// index, the map value is the generated pkScript.
286+
GeneratedOutputs map[int][]byte
287+
}
288+
280289
// BTCSign signs a bitcoin or bitcoin-like transaction. The previous transactions of the inputs
281290
// need to be provided if `BTCSignNeedsPrevTxs()` returns true.
282-
//
283-
// Returns one 64 byte signature per input.
284291
func (device *Device) BTCSign(
285292
coin messages.BTCCoin,
286293
scriptConfigs []*messages.BTCScriptConfigWithKeypath,
287294
outputScriptConfigs []*messages.BTCScriptConfigWithKeypath,
288295
tx *BTCTx,
289296
formatUnit messages.BTCSignInitRequest_FormatUnit,
290-
) ([][]byte, map[int][]byte, error) {
297+
) (*BTCSignResult, error) {
291298
generatedOutputs := map[int][]byte{}
292299
if !device.version.AtLeast(semver.NewSemVer(9, 10, 0)) {
293300
for _, sc := range scriptConfigs {
294301
if isTaproot(sc) {
295-
return nil, nil, UnsupportedError("9.10.0")
302+
return nil, UnsupportedError("9.10.0")
296303
}
297304
}
298305
}
@@ -308,11 +315,11 @@ func (device *Device) BTCSign(
308315
}
309316

310317
if containsSilentPaymentOutputs && !device.version.AtLeast(semver.NewSemVer(9, 21, 0)) {
311-
return nil, nil, UnsupportedError("9.21.0")
318+
return nil, UnsupportedError("9.21.0")
312319
}
313320

314321
if len(outputScriptConfigs) > 0 && !device.version.AtLeast(semver.NewSemVer(9, 22, 0)) {
315-
return nil, nil, UnsupportedError("9.22.0")
322+
return nil, UnsupportedError("9.22.0")
316323
}
317324

318325
signatures := make([][]byte, len(tx.Inputs))
@@ -330,7 +337,7 @@ func (device *Device) BTCSign(
330337
OutputScriptConfigs: outputScriptConfigs,
331338
}}})
332339
if err != nil {
333-
return nil, nil, err
340+
return nil, err
334341
}
335342

336343
isInputsPass2 := false
@@ -349,7 +356,7 @@ func (device *Device) BTCSign(
349356
if performAntiklepto {
350357
nonce, err := generateHostNonce()
351358
if err != nil {
352-
return nil, nil, err
359+
return nil, err
353360
}
354361
hostNonce = nonce
355362
input.HostNonceCommitment = &messages.AntiKleptoHostNonceCommitment{
@@ -361,12 +368,12 @@ func (device *Device) BTCSign(
361368
BtcSignInput: input,
362369
}})
363370
if err != nil {
364-
return nil, nil, err
371+
return nil, err
365372
}
366373

367374
if performAntiklepto {
368375
if next.Type != messages.BTCSignNextResponse_HOST_NONCE || next.AntiKleptoSignerCommitment == nil {
369-
return nil, nil, errp.New("unexpected response; expected signer nonce commitment")
376+
return nil, errp.New("unexpected response; expected signer nonce commitment")
370377
}
371378
signerCommitment := next.AntiKleptoSignerCommitment.Commitment
372379
next, err = device.nestedQueryBtcSign(
@@ -378,20 +385,20 @@ func (device *Device) BTCSign(
378385
},
379386
})
380387
if err != nil {
381-
return nil, nil, err
388+
return nil, err
382389
}
383390
err := antikleptoVerify(
384391
hostNonce,
385392
signerCommitment,
386393
next.Signature,
387394
)
388395
if err != nil {
389-
return nil, nil, err
396+
return nil, err
390397
}
391398
}
392399
if isInputsPass2 {
393400
if !next.HasSignature {
394-
return nil, nil, errp.New("unexpected response; expected signature")
401+
return nil, errp.New("unexpected response; expected signature")
395402
}
396403
signatures[inputIndex] = next.Signature
397404
}
@@ -413,7 +420,7 @@ func (device *Device) BTCSign(
413420
},
414421
})
415422
if err != nil {
416-
return nil, nil, err
423+
return nil, err
417424
}
418425
case messages.BTCSignNextResponse_PREVTX_INPUT:
419426
prevtxInput := tx.Inputs[next.Index].PrevTx.Inputs[next.PrevIndex]
@@ -424,7 +431,7 @@ func (device *Device) BTCSign(
424431
},
425432
})
426433
if err != nil {
427-
return nil, nil, err
434+
return nil, err
428435
}
429436
case messages.BTCSignNextResponse_PREVTX_OUTPUT:
430437
prevtxOutput := tx.Inputs[next.Index].PrevTx.Outputs[next.PrevIndex]
@@ -435,7 +442,7 @@ func (device *Device) BTCSign(
435442
},
436443
})
437444
if err != nil {
438-
return nil, nil, err
445+
return nil, err
439446
}
440447
case messages.BTCSignNextResponse_OUTPUT:
441448
outputIndex := next.Index
@@ -444,7 +451,7 @@ func (device *Device) BTCSign(
444451
BtcSignOutput: tx.Outputs[outputIndex],
445452
}})
446453
if err != nil {
447-
return nil, nil, err
454+
return nil, err
448455
}
449456
if next.GeneratedOutputPkscript != nil {
450457
generatedOutputs[int(outputIndex)] = next.GeneratedOutputPkscript
@@ -455,13 +462,13 @@ func (device *Device) BTCSign(
455462
next.GeneratedOutputPkscript,
456463
)
457464
if err != nil {
458-
return nil, nil, err
465+
return nil, err
459466
}
460467
}
461468
case messages.BTCSignNextResponse_PAYMENT_REQUEST:
462469
paymentRequestIndex := next.Index
463470
if int(paymentRequestIndex) >= len(tx.PaymentRequests) {
464-
return nil, nil, errp.New("payment request index out of bounds")
471+
return nil, errp.New("payment request index out of bounds")
465472
}
466473
paymentRequest := tx.PaymentRequests[paymentRequestIndex]
467474
next, err = device.nestedQueryBtcSign(
@@ -472,10 +479,13 @@ func (device *Device) BTCSign(
472479
},
473480
)
474481
if err != nil {
475-
return nil, nil, err
482+
return nil, err
476483
}
477484
case messages.BTCSignNextResponse_DONE:
478-
return signatures, generatedOutputs, nil
485+
return &BTCSignResult{
486+
Signatures: signatures,
487+
GeneratedOutputs: generatedOutputs,
488+
}, nil
479489
}
480490
}
481491
}
@@ -542,18 +552,28 @@ func (device *Device) BTCRegisterScriptConfig(
542552
return nil
543553
}
544554

555+
// BTCSignMessageResult is the result of `BTCSignMessage()`.
556+
type BTCSignMessageResult struct {
557+
// Signature is the 64 byte raw signature.
558+
Signature []byte
559+
// RecID is the recoverable ID.
560+
RecID byte
561+
// ElectrumSig65 is the 65 byte signature in Electrum format.
562+
ElectrumSig65 []byte
563+
}
564+
545565
// BTCSignMessage signs a Bitcoin message. The 64 byte raw signature, the recoverable ID and the 65
546566
// byte signature in Electrum format are returned.
547567
func (device *Device) BTCSignMessage(
548568
coin messages.BTCCoin,
549569
scriptConfig *messages.BTCScriptConfigWithKeypath,
550570
message []byte,
551-
) (raw []byte, recID byte, electrum65 []byte, err error) {
571+
) (*BTCSignMessageResult, error) {
552572
if isTaproot(scriptConfig) {
553-
return nil, 0, nil, errp.New("taproot not supported")
573+
return nil, errp.New("taproot not supported")
554574
}
555575
if !device.version.AtLeast(semver.NewSemVer(9, 2, 0)) {
556-
return nil, 0, nil, UnsupportedError("9.2.0")
576+
return nil, UnsupportedError("9.2.0")
557577
}
558578

559579
supportsAntiklepto := device.version.AtLeast(semver.NewSemVer(9, 5, 0))
@@ -564,7 +584,7 @@ func (device *Device) BTCSignMessage(
564584
var err error
565585
hostNonce, err = generateHostNonce()
566586
if err != nil {
567-
return nil, 0, nil, err
587+
return nil, err
568588
}
569589
hostNonceCommitment = &messages.AntiKleptoHostNonceCommitment{
570590
Commitment: antikleptoHostCommit(hostNonce),
@@ -583,14 +603,14 @@ func (device *Device) BTCSignMessage(
583603
}
584604
response, err := device.queryBTC(request)
585605
if err != nil {
586-
return nil, 0, nil, err
606+
return nil, err
587607
}
588608

589609
var signature []byte
590610
if supportsAntiklepto {
591611
signerCommitment, ok := response.Response.(*messages.BTCResponse_AntikleptoSignerCommitment)
592612
if !ok {
593-
return nil, 0, nil, errp.New("unexpected response")
613+
return nil, errp.New("unexpected response")
594614
}
595615
response, err := device.queryBTC(&messages.BTCRequest{
596616
Request: &messages.BTCRequest_AntikleptoSignature{
@@ -600,12 +620,12 @@ func (device *Device) BTCSignMessage(
600620
},
601621
})
602622
if err != nil {
603-
return nil, 0, nil, err
623+
return nil, err
604624
}
605625

606626
signResponse, ok := response.Response.(*messages.BTCResponse_SignMessage)
607627
if !ok {
608-
return nil, 0, nil, errp.New("unexpected response")
628+
return nil, errp.New("unexpected response")
609629
}
610630
signature = signResponse.SignMessage.Signature
611631
err = antikleptoVerify(
@@ -614,12 +634,12 @@ func (device *Device) BTCSignMessage(
614634
signature[:64],
615635
)
616636
if err != nil {
617-
return nil, 0, nil, err
637+
return nil, err
618638
}
619639
} else {
620640
signResponse, ok := response.Response.(*messages.BTCResponse_SignMessage)
621641
if !ok {
622-
return nil, 0, nil, errp.New("unexpected response")
642+
return nil, errp.New("unexpected response")
623643
}
624644
signature = signResponse.SignMessage.Signature
625645
}
@@ -628,5 +648,9 @@ func (device *Device) BTCSignMessage(
628648
// See https://github.com/spesmilo/electrum/blob/84dc181b6e7bb20e88ef6b98fb8925c5f645a765/electrum/ecc.py#L521-L523
629649
const compressed = 4 // BitBox02 uses only compressed pubkeys
630650
electrumSig65 := append([]byte{27 + compressed + recID}, sig...)
631-
return sig, recID, electrumSig65, nil
651+
return &BTCSignMessageResult{
652+
Signature: sig,
653+
RecID: recID,
654+
ElectrumSig65: electrumSig65,
655+
}, nil
632656
}

‎api/firmware/btc_test.go

+12-12
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func TestSimulatorBTCSignMessage(t *testing.T) {
161161

162162
pubKey := simulatorPub(t, device, keypath...)
163163

164-
sig, _, _, err := device.BTCSignMessage(
164+
result, err := device.BTCSignMessage(
165165
coin,
166166
&messages.BTCScriptConfigWithKeypath{
167167
ScriptConfig: NewBTCScriptConfigSimple(messages.BTCScriptConfig_P2WPKH_P2SH),
@@ -171,7 +171,7 @@ func TestSimulatorBTCSignMessage(t *testing.T) {
171171
)
172172
require.NoError(t, err)
173173
sigHash := chainhash.DoubleHashB([]byte("\x18Bitcoin Signed Message:\n\x07message"))
174-
require.True(t, parseECDSASignature(t, sig).Verify(sigHash, pubKey))
174+
require.True(t, parseECDSASignature(t, result.Signature).Verify(sigHash, pubKey))
175175
})
176176
}
177177

@@ -352,7 +352,7 @@ func TestBTCSignMessage(t *testing.T) {
352352
generateHostNonce = func() ([]byte, error) {
353353
return hostNonce, nil
354354
}
355-
sig, recID, electrumSig65, err := env.device.BTCSignMessage(
355+
result, err := env.device.BTCSignMessage(
356356
messages.BTCCoin_BTC,
357357
&messages.BTCScriptConfigWithKeypath{
358358
ScriptConfig: NewBTCScriptConfigSimple(messages.BTCScriptConfig_P2WPKH_P2SH),
@@ -362,9 +362,9 @@ func TestBTCSignMessage(t *testing.T) {
362362
)
363363
if env.version.AtLeast(semver.NewSemVer(9, 2, 0)) {
364364
require.NoError(t, err)
365-
require.Equal(t, expectedSig[:64], sig)
366-
require.Equal(t, byte(0), recID)
367-
require.Equal(t, electrumSig65, append([]byte{31}, expectedSig[:64]...))
365+
require.Equal(t, expectedSig[:64], result.Signature)
366+
require.Equal(t, byte(0), result.RecID)
367+
require.Equal(t, result.ElectrumSig65, append([]byte{31}, expectedSig[:64]...))
368368
} else {
369369
require.EqualError(t, err, UnsupportedError("9.2.0").Error())
370370
}
@@ -422,7 +422,7 @@ func TestSimulatorBTCSignTaprootKeySpend(t *testing.T) {
422422
require.False(t, BTCSignNeedsPrevTxs(scriptConfigs))
423423

424424
prevTxHash := prevTx.TxHash()
425-
_, _, err := device.BTCSign(
425+
_, err := device.BTCSign(
426426
coin,
427427
scriptConfigs,
428428
nil,
@@ -551,7 +551,7 @@ func TestSimulatorBTCSignMixed(t *testing.T) {
551551
require.True(t, BTCSignNeedsPrevTxs(scriptConfigs))
552552

553553
prevTxHash := prevTx.TxHash()
554-
_, _, err := device.BTCSign(
554+
_, err := device.BTCSign(
555555
coin,
556556
scriptConfigs,
557557
nil,
@@ -648,7 +648,7 @@ func TestSimulatorBTCSignSilentPayment(t *testing.T) {
648648
LockTime: 0,
649649
}
650650
prevTxHash := prevTx.TxHash()
651-
_, generatedOutputs, err := device.BTCSign(
651+
result, err := device.BTCSign(
652652
coin,
653653
[]*messages.BTCScriptConfigWithKeypath{
654654
{
@@ -707,7 +707,7 @@ func TestSimulatorBTCSignSilentPayment(t *testing.T) {
707707
map[int][]byte{
708708
1: unhex("5120f99b8e8d97aa7b068dd7b4e7ae31f51784f5c2a0cae280748cfd23832b7dcba7"),
709709
},
710-
generatedOutputs,
710+
result.GeneratedOutputs,
711711
)
712712
} else {
713713
require.EqualError(t, err, UnsupportedError("9.21.0").Error())
@@ -753,7 +753,7 @@ func TestSimulatorSignBTCTransactionSendSelfSameAccount(t *testing.T) {
753753
}
754754

755755
prevTxHash := prevTx.TxHash()
756-
_, _, err := device.BTCSign(
756+
_, err := device.BTCSign(
757757
coin,
758758
scriptConfigs,
759759
nil,
@@ -847,7 +847,7 @@ func TestSimulatorSignBTCTransactionSendSelfDifferentAccount(t *testing.T) {
847847
outputScriptConfigIndex := uint32(0)
848848

849849
prevTxHash := prevTx.TxHash()
850-
_, _, err := device.BTCSign(
850+
_, err := device.BTCSign(
851851
coin,
852852
scriptConfigs,
853853
outputScriptConfigs,

‎cmd/miniscript/main.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func main() {
331331
}
332332
accountKeypath := keyOriginInfo.Keypath
333333
mp := multipaths[i]
334-
signatures, _, err := device.BTCSign(
334+
result, err := device.BTCSign(
335335
coin,
336336
[]*messages.BTCScriptConfigWithKeypath{
337337
{
@@ -390,8 +390,8 @@ func main() {
390390
errpanic(err)
391391

392392
// Convert to DER encoding
393-
signaturesDER := make([][]byte, len(signatures))
394-
for sigIndex, signature := range signatures {
393+
signaturesDER := make([][]byte, len(result.Signatures))
394+
for sigIndex, signature := range result.Signatures {
395395
r := new(btcec.ModNScalar)
396396
r.SetByteSlice(signature[:32])
397397
s := new(btcec.ModNScalar)

‎cmd/paymentrequest/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func main() {
167167
errpanic(err)
168168
paymentRequest.Signature = signature[1:]
169169

170-
_, _, err = device.BTCSign(
170+
_, err = device.BTCSign(
171171
messages.BTCCoin_TBTC,
172172
[]*messages.BTCScriptConfigWithKeypath{
173173
{

0 commit comments

Comments
 (0)