BACK-1673: add more information in CreateTxResponse (#26)

As the CreateTransaction service has 2 purposes (setting the change output and serializing), we expose the "setting change output" effect in a parse-able way. We don't want to parse the RawTx again to know the change amount and the total fees for the created transaction Changes: change_amount is added to CreateTransactionResponse total_fees is added to CreateTransactionResponse
LedgerHQ · Mar 17, 2021 · d5b6717 · d5b6717
1 parent 3940257
commit d5b6717
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 19 deletions.
diff --git a/grpc/bitcoin.go b/grpc/bitcoin.go
@@ -113,21 +113,23 @@ func (c *controller) CreateTransaction(
 		return nil, status.Errorf(codes.InvalidArgument, err.Error())
 	}
 
-	rawTx, err := c.svc.CreateTransaction(tx, chainParams)
+	rawTxWithExtra, err := c.svc.CreateTransaction(tx, chainParams)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, err.Error())
 	}
 
-	var notEnoughtUtxo *pb.NotEnoughUtxo
-	if rawTx.NotEnoughUtxo != nil {
-		notEnoughtUtxo = &pb.NotEnoughUtxo{MissingAmount: rawTx.NotEnoughUtxo.MissingAmount}
+	var notEnoughUtxo *pb.NotEnoughUtxo
+	if rawTxWithExtra.RawTx.NotEnoughUtxo != nil {
+		notEnoughUtxo = &pb.NotEnoughUtxo{MissingAmount: rawTxWithExtra.RawTx.NotEnoughUtxo.MissingAmount}
 	}
 
 	response := pb.RawTransactionResponse{
-		Hex:           rawTx.Hex,
-		Hash:          rawTx.Hash,
-		WitnessHash:   rawTx.WitnessHash,
-		NotEnoughUtxo: notEnoughtUtxo,
+		Hex:           rawTxWithExtra.RawTx.Hex,
+		Hash:          rawTxWithExtra.RawTx.Hash,
+		WitnessHash:   rawTxWithExtra.RawTx.WitnessHash,
+		ChangeAmount:  rawTxWithExtra.Change,
+		TotalFees:     rawTxWithExtra.TotalFees,
+		NotEnoughUtxo: notEnoughUtxo,
 	}
 
 	return &response, nil

diff --git a/pb/bitcoin/service.proto b/pb/bitcoin/service.proto
@@ -221,7 +221,7 @@ message CreateTransactionRequest {
   int64 fee_sat_per_kb = 6;
 }
 
-// RawTransactionResponse defines the builded raw tx.
+// RawTransactionResponse defines the built raw tx.
 message RawTransactionResponse {
   // hex contains the unsigned transaction serialized according to bitcoin
   // encoding.
@@ -237,8 +237,12 @@ message RawTransactionResponse {
   // Same as hash if no witness is present.
   string witness_hash = 3;
 
+  int64 change_amount = 4;
+
+  int64 total_fees = 5;
+
   // If not enough utxos to pay for fees.
-  NotEnoughUtxo not_enough_utxo = 4;
+  NotEnoughUtxo not_enough_utxo = 6;
 }
 
 message NotEnoughUtxo {

diff --git a/pkg/core/tx.go b/pkg/core/tx.go
@@ -48,6 +48,12 @@ type RawTx struct {
 	NotEnoughUtxo *NotEnoughUtxo
 }
 
+type RawTxWithChangeFees struct {
+	RawTx     RawTx
+	Change    int64
+	TotalFees int64
+}
+
 type NotEnoughUtxo struct {
 	MissingAmount int64
 }
@@ -66,7 +72,7 @@ type SignatureMetadata struct {
 	AddrEncoding AddressEncoding
 }
 
-func (s *Service) CreateTransaction(tx *Tx, chainParams chaincfg.ChainParams) (*RawTx, error) {
+func (s *Service) CreateTransaction(tx *Tx, chainParams chaincfg.ChainParams) (*RawTxWithChangeFees, error) {
 	var inputAmount, targetAmount int64
 
 	// Create a new btcd transaction
@@ -158,7 +164,12 @@ func (s *Service) CreateTransaction(tx *Tx, chainParams chaincfg.ChainParams) (*
 
 	// Not enough utxos to pay fees
 	if changeAmount < 0 {
-		return &RawTx{NotEnoughUtxo: &NotEnoughUtxo{maxRequiredFee}}, nil
+		retval := RawTxWithChangeFees{
+			RawTx:     RawTx{NotEnoughUtxo: &NotEnoughUtxo{maxRequiredFee}},
+			Change:    changeAmount,
+			TotalFees: 0,
+		}
+		return &retval, nil
 	}
 
 	// Add change output to TxOut arrays
@@ -171,7 +182,14 @@ func (s *Service) CreateTransaction(tx *Tx, chainParams chaincfg.ChainParams) (*
 	msgTx.LockTime = tx.LockTime
 
 	// Encode MsgTx to RawTx
-	return encodeMsgTx(msgTx)
+	rawTx, err := encodeMsgTx(msgTx)
+
+	return &RawTxWithChangeFees{
+		RawTx:     *rawTx,
+		Change:    changeAmount,
+		TotalFees: inputAmount - targetAmount - changeAmount,
+	}, err
+	// encodeMsgTx(msgTx), changeAmount,
 }
 
 func (s *Service) GenerateDerSignatures(msgTx *wire.MsgTx, utxos []Utxo, privKey string) ([]DerSignature, error) {

diff --git a/pkg/core/tx_test.go b/pkg/core/tx_test.go
@@ -77,24 +77,24 @@ func TestCreateTransaction(t *testing.T) {
 				t.Fatalf("CreateTransaction() got error '%v'", err)
 			}
 
-			if rawTx.NotEnoughUtxo != nil && tt.wantNotEnoughUtxoAmount == nil {
-				t.Fatalf("CreateTransaction() got NotEnoughUtxo '%v'", rawTx.NotEnoughUtxo)
+			if rawTx.RawTx.NotEnoughUtxo != nil && tt.wantNotEnoughUtxoAmount == nil {
+				t.Fatalf("CreateTransaction() got NotEnoughUtxo '%v'", rawTx.RawTx.NotEnoughUtxo)
 			}
 
-			if rawTx.NotEnoughUtxo == nil && tt.wantNotEnoughUtxoAmount != nil {
+			if rawTx.RawTx.NotEnoughUtxo == nil && tt.wantNotEnoughUtxoAmount != nil {
 				t.Fatalf("CreateTransaction() should have '%v'", tt.wantNotEnoughUtxoAmount)
 			}
 
-			if rawTx.NotEnoughUtxo != nil && tt.wantNotEnoughUtxoAmount != nil && rawTx.NotEnoughUtxo.MissingAmount != tt.wantNotEnoughUtxoAmount.MissingAmount {
-				t.Fatalf("CreateTransaction() got NotEnoughUtxo '%v'", rawTx.NotEnoughUtxo)
+			if rawTx.RawTx.NotEnoughUtxo != nil && tt.wantNotEnoughUtxoAmount != nil && rawTx.RawTx.NotEnoughUtxo.MissingAmount != tt.wantNotEnoughUtxoAmount.MissingAmount {
+				t.Fatalf("CreateTransaction() got NotEnoughUtxo '%v'", rawTx.RawTx.NotEnoughUtxo)
 			}
 
 			if tt.wantNotEnoughUtxoAmount == nil {
 				if rawTx == nil {
 					t.Fatalf("CreateTransaction() got nil response")
 				}
 
-				if len(rawTx.Hex) == 0 {
+				if len(rawTx.RawTx.Hex) == 0 {
 					t.Fatalf("CreateTransaction() got empty raw hex")
 				}
 			}