refactor: address review comments

.changeset/add_transaction_construction_api.md

@@ -4,12 +4,6 @@ default: minor
 
 # Add transaction construction API
 
-Added two new endpoints to construct transactions with a few restrictions for ease of use. Clients can still use the existing fund endpoints for "advanced" transactions. All addresses in the wallet must all have either unlock conditions with a single required signature or a public key spend policy.
-
-This is a two step process. The private keys are never transmitted to the server.
-
-The client first calls `[POST] /api/:wallet/transaction/construct` with the recipients. The server will construct the transaction and return a list of hashes that the client needs to sign to broadcast the transaction. The client needs to match the returned public keys to their ed25519 private key and sign all of the hashes.
-
-After signing, the client calls `[POST] /api/:wallet/transaction/construct/:id` with the array of signatures. The server will add the signatures to the transaction and broadcast it. If the client provided the correct signatures, the transaction will be added to the tpool and broadcast.
+Adds two new endpoints to construct transactions. This combines and simplifies the existing fund flow for sending siacoin and siafund transactions. will be added to the tpool and broadcast.
 
 See API docs for request and response bodies

api/api_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -1365,8 +1366,21 @@ func TestConstructSiacoins(t *testing.T) {
 	testutil.MineBlocks(t, cm, types.VoidAddress, 1)
 	waitForBlock(t, cm, ws)
 
+	// try to construct a transaction with more siafunds than the wallet holds.
+	// this will lock all of the wallet's siacoins
 	resp, err := wc.Construct([]types.SiacoinOutput{
 		{Value: types.Siacoins(1), Address: receiverAddr},
+	}, []types.SiafundOutput{
+		{Value: 100000, Address: senderAddr},
+	}, senderAddr)
+	if !strings.Contains(err.Error(), "insufficient funds") {
+		t.Fatal(err)
+	}
+
+	// construct a transaction with a single siacoin output
+	// this will fail if the utxos were not unlocked
+	resp, err = wc.Construct([]types.SiacoinOutput{
+		{Value: types.Siacoins(1), Address: receiverAddr},
 	}, nil, senderAddr)
 	if err != nil {
 		t.Fatal(err)
@@ -1594,6 +1608,16 @@ func TestConstructSiafunds(t *testing.T) {
 	case sent.SiafundOutflow()-sent.SiafundInflow() != 1:
 		t.Fatalf("expected unconfirmed event to have siafund outflow of 1, got %v", sent.SiafundOutflow()-sent.SiafundInflow())
 	}
+
+	claim := confirmed[0]
+	switch {
+	case claim.Type != wallet.EventTypeSiafundClaim:
+		t.Fatalf("expected claim event to have type %q, got %q", wallet.EventTypeSiafundClaim, claim.Type)
+	case !claim.SiacoinOutflow().IsZero():
+		t.Fatalf("expected claim event to have siacoin outflow of 0, got %v", claim.SiacoinOutflow())
+	case !claim.SiacoinInflow().IsZero():
+		t.Fatalf("expected claim event to have siacoin inflow of 0, got %v", claim.SiacoinInflow())
+	}
 }
 
 func TestConstructV2Siacoins(t *testing.T) {
@@ -1675,8 +1699,21 @@ func TestConstructV2Siacoins(t *testing.T) {
 	testutil.MineBlocks(t, cm, types.VoidAddress, 1)
 	waitForBlock(t, cm, ws)
 
+	// try to construct a transaction with more siafunds than the wallet holds.
+	// this will lock all of the wallet's Siacoin UTXOs
 	resp, err := wc.ConstructV2([]types.SiacoinOutput{
 		{Value: types.Siacoins(1), Address: receiverAddr},
+	}, []types.SiafundOutput{
+		{Value: 100000, Address: senderAddr},
+	}, senderAddr)
+	if !strings.Contains(err.Error(), "insufficient funds") {
+		t.Fatal(err)
+	}
+
+	// this will fail if the utxos were not properly
+	// unlocked when the previous request failed
+	resp, err = wc.ConstructV2([]types.SiacoinOutput{
+		{Value: types.Siacoins(1), Address: receiverAddr},
 	}, nil, senderAddr)
 	if err != nil {
 		t.Fatal(err)
@@ -1890,6 +1927,16 @@ func TestConstructV2Siafunds(t *testing.T) {
 	case sent.SiafundOutflow()-sent.SiafundInflow() != 1:
 		t.Fatalf("expected unconfirmed event to have siafund outflow of 1, got %v", sent.SiafundOutflow()-sent.SiafundInflow())
 	}
+
+	claim := confirmed[0]
+	switch {
+	case claim.Type != wallet.EventTypeSiafundClaim:
+		t.Fatalf("expected claim event to have type %q, got %q", wallet.EventTypeSiafundClaim, claim.Type)
+	case !claim.SiacoinOutflow().IsZero():
+		t.Fatalf("expected claim event to have siacoin outflow of 0, got %v", claim.SiacoinOutflow())
+	case !claim.SiacoinInflow().IsZero():
+		t.Fatalf("expected claim event to have siacoin inflow of 0, got %v", claim.SiacoinInflow())
+	}
 }
 
 func TestDebugMine(t *testing.T) {

api/client.go

@@ -338,7 +338,8 @@ func (c *WalletClient) FundSF(txn types.Transaction, amount uint64, changeAddr,
 	return
 }
 
-// Construct constructs a transaction and returns its ID
+// Construct constructs a transaction sending the specified Siacoins or Siafunds to the recipients. The transaction is returned
+// along with its ID and calculated miner fee. The transaction will need to be signed before broadcasting.
 func (c *WalletClient) Construct(siacoins []types.SiacoinOutput, siafunds []types.SiafundOutput, change types.Address) (resp WalletConstructResponse, err error) {
 	err = c.c.POST(fmt.Sprintf("/wallets/%v/construct/transaction", c.id), WalletConstructRequest{
 		Siacoins:      siacoins,
@@ -348,7 +349,8 @@ func (c *WalletClient) Construct(siacoins []types.SiacoinOutput, siafunds []type
 	return
 }
 
-// ConstructV2 constructs a v2 transaction and returns its ID
+// Construct constructs a V2 transaction sending the specified Siacoins or Siafunds to the recipients. The transaction is returned
+// along with its ID and calculated miner fee. The transaction will need to be signed before broadcasting.
 func (c *WalletClient) ConstructV2(siacoins []types.SiacoinOutput, siafunds []types.SiafundOutput, change types.Address) (resp WalletConstructV2Response, err error) {
 	err = c.c.POST(fmt.Sprintf("/wallets/%v/construct/v2/transaction", c.id), WalletConstructRequest{
 		Siacoins:      siacoins,

api/server.go

@@ -686,6 +686,15 @@ func (s *server) walletsConstructHandler(jc jape.Context) {
 	if err := jc.DecodeParam("id", &walletID); err != nil {
 		return
 	}
+
+	_, err := s.wm.WalletBalance(walletID)
+	if errors.Is(err, wallet.ErrNotFound) {
+		jc.Error(err, http.StatusNotFound)
+		return
+	} else if jc.Check("failed to get wallet", err) != nil {
+		return
+	}
+
 	var wcr WalletConstructRequest
 	if err := jc.Decode(&wcr); err != nil {
 		return
@@ -775,7 +784,6 @@ func (s *server) walletsConstructHandler(jc jape.Context) {
 			return a, nil
 		}
 		a, err := s.wm.WalletAddress(walletID, addr)
-
 		if err != nil {
 			return wallet.Address{}, err
 		}
@@ -871,11 +879,25 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 	if err := jc.DecodeParam("id", &walletID); err != nil {
 		return
 	}
+
+	_, err := s.wm.WalletBalance(walletID)
+	if errors.Is(err, wallet.ErrNotFound) {
+		jc.Error(err, http.StatusNotFound)
+		return
+	} else if jc.Check("failed to get wallet", err) != nil {
+		return
+	}
+
 	var wcr WalletConstructRequest
 	if err := jc.Decode(&wcr); err != nil {
 		return
 	}
 
+	if wcr.ChangeAddress == types.VoidAddress {
+		jc.Error(errors.New("change address must be specified"), http.StatusBadRequest)
+		return
+	}
+
 	var siacoinInput types.Currency
 	for i, sco := range wcr.Siacoins {
 		switch {
@@ -927,11 +949,6 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 	}
 
 	if !siacoinChange.IsZero() {
-		if wcr.ChangeAddress == types.VoidAddress {
-			jc.Error(errors.New("change address must be specified"), http.StatusBadRequest)
-			return
-		}
-
 		wcr.Siacoins = append(wcr.Siacoins, types.SiacoinOutput{
 			Value:   siacoinChange,
 			Address: wcr.ChangeAddress,
@@ -948,11 +965,6 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 	}
 
 	if siafundChange > 0 {
-		if wcr.ChangeAddress == types.VoidAddress {
-			jc.Error(errors.New("change address must be specified"), http.StatusBadRequest)
-			return
-		}
-
 		wcr.Siafunds = append(wcr.Siafunds, types.SiafundOutput{
 			Value:   siafundChange,
 			Address: wcr.ChangeAddress,
@@ -965,7 +977,6 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 			return a, nil
 		}
 		a, err := s.wm.WalletAddress(walletID, addr)
-
 		if err != nil {
 			return wallet.Address{}, err
 		}
@@ -1025,9 +1036,6 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 
 		sci := types.V2SiacoinInput{
 			Parent: sce,
-			SatisfiedPolicy: types.SatisfiedPolicy{
-				Policy: *addr.SpendPolicy,
-			},
 		}
 
 		if addr.SpendPolicy != nil {
@@ -1036,7 +1044,6 @@ func (s *server) walletsConstructV2Handler(jc jape.Context) {
 				Policy: *addr.SpendPolicy,
 			}
 		}
-
 		txn.SiacoinInputs = append(txn.SiacoinInputs, sci)
 	}