From fc9c315c1600a815c4cf6f1f910b5c64db21cff9 Mon Sep 17 00:00:00 2001 From: Nate Date: Fri, 10 Jan 2025 09:35:27 -0800 Subject: [PATCH] refactor: address review comments --- .../add_transaction_construction_api.md | 8 +--- api/api_test.go | 47 +++++++++++++++++++ api/client.go | 6 ++- api/server.go | 39 ++++++++------- 4 files changed, 75 insertions(+), 25 deletions(-) diff --git a/.changeset/add_transaction_construction_api.md b/.changeset/add_transaction_construction_api.md index 18f82df..f598024 100644 --- a/.changeset/add_transaction_construction_api.md +++ b/.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 \ No newline at end of file diff --git a/api/api_test.go b/api/api_test.go index 486f255..63b086c 100644 --- a/api/api_test.go +++ b/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) { diff --git a/api/client.go b/api/client.go index 0b55a84..518ed8c 100644 --- a/api/client.go +++ b/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, diff --git a/api/server.go b/api/server.go index 17c4bd0..6ebadf2 100644 --- a/api/server.go +++ b/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) }