From fedbb3bd0fb493b7c616ad2f3970db1b7379f4e4 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Thu, 12 Sep 2024 14:19:13 +0300 Subject: [PATCH] Some fixes --- .gitignore | 1 + cmd/kaspawallet/create_unsigned_tx.go | 9 ++- cmd/kaspawallet/daemon/client/client.go | 3 +- cmd/kaspawallet/daemon/pb/kaspawalletd.proto | 1 + .../daemon/server/broadcast_rbf.go | 18 ++++- cmd/kaspawallet/daemon/server/bump_fee.go | 73 ++++++++++--------- .../server/create_unsigned_transaction.go | 40 +++++++--- .../daemon/server/split_transaction.go | 14 ++-- cmd/kaspawallet/libkaspawallet/transaction.go | 7 ++ .../rpc_submit_transaction_replacement.go | 18 +++-- 10 files changed, 123 insertions(+), 61 deletions(-) diff --git a/.gitignore b/.gitignore index 0fa51a09d..964249db3 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,7 @@ _testmain.go debug debug.test __debug_bin +*__debug_* # CI version.txt diff --git a/cmd/kaspawallet/create_unsigned_tx.go b/cmd/kaspawallet/create_unsigned_tx.go index 93c0c729d..5bf8ec06a 100644 --- a/cmd/kaspawallet/create_unsigned_tx.go +++ b/cmd/kaspawallet/create_unsigned_tx.go @@ -21,10 +21,13 @@ func createUnsignedTransaction(conf *createUnsignedTransactionConfig) error { ctx, cancel := context.WithTimeout(context.Background(), daemonTimeout) defer cancel() - sendAmountSompi, err := utils.KasToSompi(conf.SendAmount) + var sendAmountSompi uint64 - if err != nil { - return err + if !conf.IsSendAll { + sendAmountSompi, err = utils.KasToSompi(conf.SendAmount) + if err != nil { + return err + } } feeRate := &pb.FeeRate{ diff --git a/cmd/kaspawallet/daemon/client/client.go b/cmd/kaspawallet/daemon/client/client.go index bc25753ed..a28293510 100644 --- a/cmd/kaspawallet/daemon/client/client.go +++ b/cmd/kaspawallet/daemon/client/client.go @@ -2,9 +2,10 @@ package client import ( "context" - "github.com/kaspanet/kaspad/cmd/kaspawallet/daemon/server" "time" + "github.com/kaspanet/kaspad/cmd/kaspawallet/daemon/server" + "github.com/pkg/errors" "github.com/kaspanet/kaspad/cmd/kaspawallet/daemon/pb" diff --git a/cmd/kaspawallet/daemon/pb/kaspawalletd.proto b/cmd/kaspawallet/daemon/pb/kaspawalletd.proto index 309c674b4..c3a55058f 100644 --- a/cmd/kaspawallet/daemon/pb/kaspawalletd.proto +++ b/cmd/kaspawallet/daemon/pb/kaspawalletd.proto @@ -13,6 +13,7 @@ service kaspawalletd { rpc NewAddress(NewAddressRequest) returns (NewAddressResponse) {} rpc Shutdown(ShutdownRequest) returns (ShutdownResponse) {} rpc Broadcast(BroadcastRequest) returns (BroadcastResponse) {} + // BroadcastRBF assumes that all transactions depends on the first one rpc BroadcastRBF(BroadcastRequest) returns (BroadcastResponse) {} // Since SendRequest contains a password - this command should only be used on // a trusted or secure connection diff --git a/cmd/kaspawallet/daemon/server/broadcast_rbf.go b/cmd/kaspawallet/daemon/server/broadcast_rbf.go index e7e81a389..4cc0e0632 100644 --- a/cmd/kaspawallet/daemon/server/broadcast_rbf.go +++ b/cmd/kaspawallet/daemon/server/broadcast_rbf.go @@ -26,6 +26,7 @@ func (s *server) BroadcastRBF(_ context.Context, request *pb.BroadcastRequest) ( return &pb.BroadcastResponse{TxIDs: txIDs}, nil } +// broadcastRBF assumes that all transactions depends on the first one func (s *server) broadcastRBF(transactions [][]byte, isDomain bool) ([]string, error) { txIDs := make([]string, len(transactions)) @@ -46,9 +47,20 @@ func (s *server) broadcastRBF(transactions [][]byte, isDomain bool) ([]string, e } } - txIDs[i], err = sendTransactionRBF(s.rpcClient, tx) - if err != nil { - return nil, err + // Once the first transaction is added to the mempool, the transactions that depend + // on the replaced transaction will be removed, so there's no need to submit them + // as RBF transactions. + if i == 0 { + txIDs[i], err = sendTransactionRBF(s.rpcClient, tx) + if err != nil { + return nil, err + } + } else { + txIDs[i], err = sendTransaction(s.rpcClient, tx) + if err != nil { + return nil, err + } + } for _, input := range tx.Inputs { diff --git a/cmd/kaspawallet/daemon/server/bump_fee.go b/cmd/kaspawallet/daemon/server/bump_fee.go index a0c8810c1..3466daf15 100644 --- a/cmd/kaspawallet/daemon/server/bump_fee.go +++ b/cmd/kaspawallet/daemon/server/bump_fee.go @@ -25,6 +25,40 @@ func (s *server) BumpFee(_ context.Context, request *pb.BumpFeeRequest) (*pb.Bum return nil, err } + outpointsToInputs := make(map[externalapi.DomainOutpoint]*externalapi.DomainTransactionInput) + var maxUTXO *walletUTXO + for _, input := range domainTx.Inputs { + outpointsToInputs[input.PreviousOutpoint] = input + utxo, ok := s.mempoolExcludedUTXOs[input.PreviousOutpoint] + if !ok { + continue + } + + input.UTXOEntry = utxo.UTXOEntry + if maxUTXO == nil || utxo.UTXOEntry.Amount() > maxUTXO.UTXOEntry.Amount() { + maxUTXO = utxo + } + } + + if maxUTXO == nil { + // If we got here it means for some reason s.mempoolExcludedUTXOs is not up to date and we need to search for the UTXOs in s.utxosSortedByAmount + for _, utxo := range s.utxosSortedByAmount { + input, ok := outpointsToInputs[*utxo.Outpoint] + if !ok { + continue + } + + input.UTXOEntry = utxo.UTXOEntry + if maxUTXO == nil || utxo.UTXOEntry.Amount() > maxUTXO.UTXOEntry.Amount() { + maxUTXO = utxo + } + } + } + + if maxUTXO == nil { + return nil, errors.Errorf("no UTXOs were found for transaction %s. This probably means the transaction is already accepted", request.TxID) + } + mass := s.txMassCalculator.CalculateTransactionOverallMass(domainTx) feeRate := float64(entry.Entry.Fee) / float64(mass) newFeeRate, err := s.calculateFeeRate(request.FeeRate) @@ -36,37 +70,6 @@ func (s *server) BumpFee(_ context.Context, request *pb.BumpFeeRequest) (*pb.Bum return nil, errors.Errorf("new fee rate (%f) is not higher than the current fee rate (%f)", newFeeRate, feeRate) } - outpointsSet := make(map[externalapi.DomainOutpoint]struct{}) - var maxUTXO *walletUTXO - for _, input := range domainTx.Inputs { - outpointsSet[input.PreviousOutpoint] = struct{}{} - utxo, ok := s.mempoolExcludedUTXOs[input.PreviousOutpoint] - if !ok { - continue - } - - if maxUTXO == nil || utxo.UTXOEntry.Amount() > maxUTXO.UTXOEntry.Amount() { - maxUTXO = utxo - } - } - - if maxUTXO == nil { - // If we got here it means for some reason s.mempoolExcludedUTXOs is not up to date and we need to search for the UTXOs in s.utxosSortedByAmount - for _, utxo := range s.utxosSortedByAmount { - if _, ok := outpointsSet[*utxo.Outpoint]; !ok { - continue - } - - if maxUTXO == nil || utxo.UTXOEntry.Amount() > maxUTXO.UTXOEntry.Amount() { - maxUTXO = utxo - } - } - } - - if maxUTXO == nil { - return nil, errors.Errorf("no UTXOs were found for transaction %s. This probably means the transaction is already accepted", request.TxID) - } - if len(domainTx.Outputs) == 0 || len(domainTx.Outputs) > 2 { return nil, errors.Errorf("kaspawallet supports only transactions with 1 or 2 outputs in transaction %s, but this transaction got %d", request.TxID, len(domainTx.Outputs)) } @@ -80,7 +83,11 @@ func (s *server) BumpFee(_ context.Context, request *pb.BumpFeeRequest) (*pb.Bum fromAddresses = append(fromAddresses, fromAddress) } - selectedUTXOs, spendValue, changeSompi, err := s.selectUTXOsWithPreselected([]*walletUTXO{maxUTXO}, outpointsSet, domainTx.Outputs[0].Value, false, newFeeRate, fromAddresses) + allowUsed := make(map[externalapi.DomainOutpoint]struct{}) + for outpoint := range outpointsToInputs { + allowUsed[outpoint] = struct{}{} + } + selectedUTXOs, spendValue, changeSompi, err := s.selectUTXOsWithPreselected([]*walletUTXO{maxUTXO}, allowUsed, domainTx.Outputs[0].Value, false, newFeeRate, fromAddresses) if err != nil { return nil, err } @@ -121,7 +128,7 @@ func (s *server) BumpFee(_ context.Context, request *pb.BumpFeeRequest) (*pb.Bum return nil, err } - unsignedTransactions, err := s.maybeAutoCompoundTransaction(unsignedTransaction, toAddress, changeAddress, changeWalletAddress, feeRate) + unsignedTransactions, err := s.maybeAutoCompoundTransaction(unsignedTransaction, toAddress, changeAddress, changeWalletAddress, newFeeRate) if err != nil { return nil, err } diff --git a/cmd/kaspawallet/daemon/server/create_unsigned_transaction.go b/cmd/kaspawallet/daemon/server/create_unsigned_transaction.go index a1f88be8e..02617b97c 100644 --- a/cmd/kaspawallet/daemon/server/create_unsigned_transaction.go +++ b/cmd/kaspawallet/daemon/server/create_unsigned_transaction.go @@ -164,7 +164,8 @@ func (s *server) selectUTXOsWithPreselected(preSelectedUTXOs []*walletUTXO, allo totalValue += utxo.UTXOEntry.Amount() - fee, err = s.estimateFee(selectedUTXOs, feeRate) + // We're overestimating a bit by assuming that any transaction will have a change output + fee, err = s.estimateFee(selectedUTXOs, feeRate, true) if err != nil { return false, err } @@ -223,23 +224,40 @@ func (s *server) selectUTXOsWithPreselected(preSelectedUTXOs []*walletUTXO, allo return selectedUTXOs, totalReceived, totalValue - totalSpend, nil } -func (s *server) estimateFee(selectedUTXOs []*libkaspawallet.UTXO, feeRate float64) (uint64, error) { +func (s *server) estimateFee(selectedUTXOs []*libkaspawallet.UTXO, feeRate float64, assumeChange bool) (uint64, error) { fakePubKey := [util.PublicKeySize]byte{} fakeAddr, err := util.NewAddressPublicKey(fakePubKey[:], s.params.Prefix) if err != nil { return 0, err } - mockPayments := []*libkaspawallet.Payment{ - { - Address: fakeAddr, - Amount: 1, - }, - { // We're overestimating a bit by assuming that any transaction will have a change output - Address: fakeAddr, - Amount: 1, - }, + totalValue := uint64(0) + for _, utxo := range selectedUTXOs { + totalValue += utxo.UTXOEntry.Amount() } + + // This is an approximation for the distribution of value between the recipient output and the change output. + var mockPayments []*libkaspawallet.Payment + if assumeChange { + mockPayments = []*libkaspawallet.Payment{ + { + Address: fakeAddr, + Amount: totalValue * 99 / 100, + }, + { + Address: fakeAddr, + Amount: totalValue / 100, + }, + } + } else { + mockPayments = []*libkaspawallet.Payment{ + { + Address: fakeAddr, + Amount: totalValue, + }, + } + } + mockTx, err := libkaspawallet.CreateUnsignedTransaction(s.keysFile.ExtendedPublicKeys, s.keysFile.MinimumSignatures, mockPayments, selectedUTXOs) diff --git a/cmd/kaspawallet/daemon/server/split_transaction.go b/cmd/kaspawallet/daemon/server/split_transaction.go index 8f08830ee..330eaf875 100644 --- a/cmd/kaspawallet/daemon/server/split_transaction.go +++ b/cmd/kaspawallet/daemon/server/split_transaction.go @@ -69,7 +69,8 @@ func (s *server) mergeTransaction( } totalValue += output.Value } - fee, err := s.estimateFee(utxos, feeRate) + // We're overestimating a bit by assuming that any transaction will have a change output + fee, err := s.estimateFee(utxos, feeRate, true) if err != nil { return nil, err } @@ -204,12 +205,15 @@ func (s *server) createSplitTransaction(transaction *serialization.PartiallySign totalSompi += selectedUTXOs[i-startIndex].UTXOEntry.Amount() } - fee, err := s.estimateFee(selectedUTXOs, feeRate) - if err != nil { - return nil, err + if len(selectedUTXOs) != 0 { + fee, err := s.estimateFee(selectedUTXOs, feeRate, false) + if err != nil { + return nil, err + } + + totalSompi -= fee } - totalSompi -= fee return libkaspawallet.CreateUnsignedTransaction(s.keysFile.ExtendedPublicKeys, s.keysFile.MinimumSignatures, []*libkaspawallet.Payment{{ diff --git a/cmd/kaspawallet/libkaspawallet/transaction.go b/cmd/kaspawallet/libkaspawallet/transaction.go index c919b5887..6d2468c89 100644 --- a/cmd/kaspawallet/libkaspawallet/transaction.go +++ b/cmd/kaspawallet/libkaspawallet/transaction.go @@ -7,6 +7,7 @@ import ( "github.com/kaspanet/kaspad/domain/consensus/utils/constants" "github.com/kaspanet/kaspad/domain/consensus/utils/subnetworks" "github.com/kaspanet/kaspad/domain/consensus/utils/txscript" + "github.com/kaspanet/kaspad/domain/consensus/utils/utxo" "github.com/kaspanet/kaspad/util" "github.com/pkg/errors" ) @@ -242,6 +243,12 @@ func ExtractTransactionDeserialized(partiallySignedTransaction *serialization.Pa } partiallySignedTransaction.Tx.Inputs[i].SignatureScript = sigScript } + partiallySignedTransaction.Tx.Inputs[i].UTXOEntry = utxo.NewUTXOEntry( + input.PrevOutput.Value, + input.PrevOutput.ScriptPublicKey, + false, // This is a fake value + 0, // This is a fake value + ) } return partiallySignedTransaction.Tx, nil } diff --git a/infrastructure/network/netadapter/server/grpcserver/protowire/rpc_submit_transaction_replacement.go b/infrastructure/network/netadapter/server/grpcserver/protowire/rpc_submit_transaction_replacement.go index 2e8cf8b5b..d3aa315c3 100644 --- a/infrastructure/network/netadapter/server/grpcserver/protowire/rpc_submit_transaction_replacement.go +++ b/infrastructure/network/netadapter/server/grpcserver/protowire/rpc_submit_transaction_replacement.go @@ -60,11 +60,20 @@ func (x *SubmitTransactionReplacementResponseMessage) toAppMessage() (appmessage if x == nil { return nil, errors.Wrapf(errorNil, "SubmitTransactionReplacementResponseMessage is nil") } - rpcErr, err := x.Error.toAppMessage() - // Error is an optional field - if err != nil && !errors.Is(err, errorNil) { - return nil, err + + if x.Error != nil { + rpcErr, err := x.Error.toAppMessage() + // Error is an optional field + if err != nil && !errors.Is(err, errorNil) { + return nil, err + } + + return &appmessage.SubmitTransactionReplacementResponseMessage{ + TransactionID: x.TransactionId, + Error: rpcErr, + }, nil } + replacedTx, err := x.ReplacedTransaction.toAppMessage() if err != nil { return nil, err @@ -72,6 +81,5 @@ func (x *SubmitTransactionReplacementResponseMessage) toAppMessage() (appmessage return &appmessage.SubmitTransactionReplacementResponseMessage{ TransactionID: x.TransactionId, ReplacedTransaction: replacedTx, - Error: rpcErr, }, nil }