Repository navigation
mobile: hex-decode claim details before signing a submarine swap claim - #9
Conversation
20115be to
016d625
Compare
kaloudis
left a comment
There was a problem hiding this comment.
Ready to merge. I checked the fix against boltz-client v2.9.0: MusigSession.Sign requires a 32-byte hash and a 66-byte nonce, and SwapClaimDetails decodes both fields from hex, so decoding them here is correct.
Verified with a test that plays Boltz's side of the signing session. It serves /v2/swap/submarine/{id}/claim from a local HTTP server, passes the hash and nonce as hex (as the host sends them), combines our partial signature with Boltz's, and checks the final signature against the swap's tweaked output key. On the base branch it fails with could not create partial signature: invalid hash length 64. On this branch it passes. Please add it to the PR as mobile/swaps_test.go:
mobile/swaps_test.go
//go:build mobile
// +build mobile
package lndmobile
import (
"bytes"
"crypto/rand"
"encoding/hex"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"github.com/BoltzExchange/boltz-client/v2/pkg/boltz"
"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/schnorr"
"github.com/btcsuite/btcd/btcec/v2/schnorr/musig2"
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/btcsuite/btcd/txscript"
"github.com/stretchr/testify/require"
)
func TestCreateClaimTransactionHexDetails(t *testing.T) {
userKey, _ := btcec.NewPrivateKey()
boltzKey, _ := btcec.NewPrivateKey()
claimLeaf := "a914" + hex.EncodeToString(make([]byte, 20)) + "87"
refundLeaf := "51"
// Boltz side of the session, tweaked the same way SwapTree.Init does.
keys := []*btcec.PublicKey{boltzKey.PubKey(), userKey.PubKey()}
internal, _, _, err := musig2.AggregateKeys(keys, false)
require.NoError(t, err)
root := txscript.AssembleTaprootScriptTree(
leaf(claimLeaf), leaf(refundLeaf),
).RootNode.TapHash()
tweak := musig2.KeyTweakDesc{
Tweak: *chainhash.TaggedHash(
chainhash.TagTapTweak,
schnorr.SerializePubKey(internal.FinalKey), root[:],
),
IsXOnly: true,
}
ctx, err := musig2.NewContext(
boltzKey, false, musig2.WithTweakedContext(tweak),
musig2.WithKnownSigners(keys),
)
require.NoError(t, err)
boltzSession, err := ctx.NewSession()
require.NoError(t, err)
boltzNonce := boltzSession.PublicNonce()
var hash [32]byte
_, _ = rand.Read(hash[:])
var got boltz.PartialSignature
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, "/v2/swap/submarine/id/claim", r.URL.Path)
require.NoError(t, json.NewDecoder(r.Body).Decode(&got))
_, _ = w.Write([]byte("{}"))
},
))
defer srv.Close()
err = CreateClaimTransaction(
srv.URL, "id", claimLeaf, refundLeaf,
hex.EncodeToString(userKey.Serialize()),
hex.EncodeToString(boltzKey.PubKey().SerializeCompressed()),
hex.EncodeToString(hash[:]), hex.EncodeToString(boltzNonce[:]),
)
require.NoError(t, err)
// Boltz combines the user's partial signature with its own; the result
// must be a valid key-path signature for the swap's output key.
_, err = boltzSession.RegisterPubNonce([66]byte(got.PubNonce))
require.NoError(t, err)
_, err = boltzSession.Sign(hash)
require.NoError(t, err)
var userSig musig2.PartialSignature
require.NoError(t, userSig.Decode(bytes.NewReader(got.PartialSignature)))
done, err := boltzSession.CombineSig(&userSig)
require.NoError(t, err)
require.True(t, done)
outputKey, _, _, err := musig2.AggregateKeys(
keys, false, musig2.WithKeyTweaks(tweak),
)
require.NoError(t, err)
require.True(t, boltzSession.FinalSig().Verify(hash[:], outputKey.FinalKey))
// Malformed hex is rejected before signing.
err = CreateClaimTransaction(
srv.URL, "id", claimLeaf, refundLeaf,
hex.EncodeToString(userKey.Serialize()),
hex.EncodeToString(boltzKey.PubKey().SerializeCompressed()),
"zz", hex.EncodeToString(boltzNonce[:]),
)
require.ErrorContains(t, err, "transaction hash")
}Run with the generated mobile stubs and -tags "mobile autopilotrpc chainrpc invoicesrpc neutrinorpc peersrpc signrpc wtclientrpc watchtowerrpc routerrpc walletrpc verrpc kvdb_sqlite".
Other notes:
- Now that signing works,
CreateClaimTransactionwill sign whatever hash the host sends, and it never sees the preimage. The only check that Boltz paid our invoice isvalidatePreimagein the Zeus app (views/Swaps/SwapDetails.tsx), which runs before the only current caller. Not a blocker, but any new caller (background claims, rescue) must keep that check. A safer design would pass the preimage and the invoice payment hash into this function and verify them here, as boltz-client's own claim code does. - Next to this change: if
privateKeyfails to hex-decode, the error is only printed withfmt.Printfand the function continues with an empty key. Same inCreateReverseClaimTransaction. Returning the error is a two-line fix that could go in this PR.
016d625 to
ea1ec7e
Compare
|
Rebased onto Lint is fixed on The branch was force-pushed, so reset your local copy before pushing again: |
4b127b6 to
6666ebf
Compare
ea1ec7e to
45bad29
Compare
6666ebf to
170fc03
Compare
45bad29 to
648b46f
Compare
Play the swap host's side of MuSig2 in an httptest server: hand out
the host's public nonce, take the client's posted partial signature,
sign, and combine the two. CreateClaimTransaction is given the hash
and nonce as hex, as the host's GET /swap/submarine/{id}/claim returns
them, and the combined signature must verify against the swap's
tweaked output key. Before the hex decoding, Sign rejected the
64-character hash with "invalid hash length 64".
Also check that malformed hex in either field fails before anything
is posted to the host.
|
Added the test as
|
e77cc86
into
ZeusLN:lnd-v0.21.4-beta-zeus.1
- mobile: hex-decode the claim details before signing a submarine cooperative claim (ZeusLN/lnd#9, #4938) - mobile: add BuildReverseClaimTransaction and BuildRefundTransaction, which return the signed transaction without broadcasting it; the existing Create* functions are unchanged (ZeusLN/lnd#10, for #4939) - lint, itest and CI fixes in the Zeus series No app changes: Zeus keeps calling the Create* functions, whose signatures are unchanged. The only proto difference from the previous release is three comments in walletkit.proto.
… claims Native code built a reverse swap claim or a submarine swap refund and broadcast it to a hardcoded mempool.space URL, mainnet or testnet3 only, keeping nothing when that failed. A cooperative claim sends the preimage to the host first, and the host settles the hold invoice as soon as it has it. If the broadcast then failed, reopening the swap saw invoice.settled and never tried again, and the host could take the lockup back at the timeout (ZeusLN#4939). Signet and regtest broadcasts went to the wrong network. Use the new BuildReverseClaimTransaction and BuildRefundTransaction from ZeusLN/lnd, which return the signed transaction, and pass a network name so regtest gets its own parameters. SwapStore now broadcasts swap transactions itself: to the user's mempool instance first, then to the swap host's /chain/BTC/transaction, counting a transaction already in the mempool or a block as broadcast. For a reverse swap claim: - the signed claim is stored on the swap before its first broadcast, and its txid once a broadcast succeeds - a failed broadcast is retried with the same transaction instead of building, and possibly revealing the preimage, again - a stored claim is broadcast again on the next visit without rechecking or rebuilding; a swap with a recorded txid is left alone - on invoice.settled without a recorded broadcast, a stored claim is sent again, or the lockup is fetched from the host and claimed through the usual checks. A lockup already spent at that point is taken as done rather than reported as a failed check. Refunds are built natively and broadcast through the same path. Needs a Lndmobile build with ZeusLN/lnd#9 and the Build* functions; fetch-libraries pins are updated once that release exists. Closes ZeusLN#4939
Fixes ZeusLN/zeus#4938 (this repo has issues disabled).
CreateClaimTransactionpassed the hextransactionHashandpubNoncefrom the host'sGET /swap/submarine/{id}/claimresponse tosession.Signas[]byte(...). That gives 64 and 132 bytes, and boltz-client'sMusigSession.Signonly accepts 32 and 66, so every cooperative submarine claim failed withinvalid hash length 64and the host had to claim on the script path.Both values are now hex-decoded before signing, the same way boltz-client's own
SwapClaimDetails(HexString) decodes them.Related
/v2/v2/...claim requests). Both are needed for a cooperative submarine claim to go through.mobile/swaps.go(againstlnd-v0.20.1-beta-zeus.1); whichever lands second needs a rebase.