fix(client): stop signing on GetRawData failure; keep RecoverPubkey pure

pkg/client/transaction/controller.go

@@ -112,7 +112,13 @@ func (C *Controller) hardwareSignTxForSending() {
 	if C.executionError != nil {
 		return
 	}
-	data, _ := C.GetRawData()
+	data, err := C.GetRawData()
+	if err != nil {
+		// Do not sign a nil/empty payload when GetRawData fails — that would
+		// produce a signature over something other than the approved transaction.
+		C.executionError = fmt.Errorf("get raw data for ledger signing: %w", err)
+		return
+	}
 	signature, err := ledger.SignTx(data)
 	if err != nil {
 		C.executionError = err
@@ -142,7 +148,7 @@ func (C *Controller) txConfirmation() {
 	if C.Behavior.ConfirmationWaitTime > 0 {
 		txHash, err := C.TransactionHash()
 		if err != nil {
-			C.executionError = fmt.Errorf("could not get tx hash")
+			C.executionError = fmt.Errorf("could not get tx hash: %w", err)
 			return
 		}
 		//fmt.Printf("TX hash: %s\nWaiting for confirmation....", txHash)
@@ -195,7 +201,22 @@ func (C *Controller) ExecuteTransaction() error {
 
 // GetRawData Byes from Transaction
 func (C *Controller) GetRawData() ([]byte, error) {
-	return proto.Marshal(C.tx.GetRawData())
+	if C.tx == nil {
+		return nil, errors.New("transaction is nil")
+	}
+	rawTransaction := C.tx.GetRawData()
+	if rawTransaction == nil {
+		return nil, errors.New("transaction raw data is nil")
+	}
+	rawData, err := proto.Marshal(rawTransaction)
+	if err != nil {
+		return nil, err
+	}
+	// Empty wire encoding is not a valid TRON payload to hash or sign.
+	if len(rawData) == 0 {
+		return nil, errors.New("transaction raw data is empty")
+	}
+	return rawData, nil
 }
 
 func (C *Controller) sendSignedTx() {

pkg/client/transaction/controller_test.go

@@ -313,15 +313,14 @@ func TestTransactionHashDeterministic(t *testing.T) {
 }
 
 func TestTransactionHashNilRawData(t *testing.T) {
+	// Previously hashed empty marshal output silently; now rejected as invalid.
 	tx := &core.Transaction{}
 	ctrl := NewController(nil, nil, nil, tx)
 
 	hash, err := ctrl.TransactionHash()
-	require.NoError(t, err, "unexpected error for nil raw data")
-	// SHA256 of empty bytes (proto.Marshal of nil RawData returns empty slice)
-	emptyHash := sha256.Sum256(nil)
-	want := common.BytesToHexString(emptyHash[:])
-	assert.Equal(t, want, hash, "nil raw data hash mismatch")
+	require.Error(t, err, "nil raw data must not produce a hash")
+	assert.Empty(t, hash)
+	assert.Contains(t, err.Error(), "transaction raw data is nil")
 }
 
 func TestTransactionHashHexFormat(t *testing.T) {
@@ -387,6 +386,80 @@ func TestSignTxForSending_UnlockedSuccess(t *testing.T) {
 	assert.Len(t, ctrl.tx.GetSignature(), 1, "expected 1 signature")
 }
 
+// ---------------------------------------------------------------------------
+// 2b. hardwareSignTxForSending / GetRawData error propagation (W50)
+// ---------------------------------------------------------------------------
+
+func TestHardwareSignTxForSending_SkipsOnExecutionError(t *testing.T) {
+	tx := newTestTransaction()
+	ctrl := NewController(nil, nil, nil, tx)
+	ctrl.executionError = errors.New("prior error")
+
+	ctrl.hardwareSignTxForSending()
+
+	assert.Empty(t, ctrl.tx.GetSignature(), "expected no signature when executionError is set")
+	assert.Equal(t, "prior error", ctrl.executionError.Error(), "executionError changed")
+}
+
+func TestHardwareSignTxForSending_PropagatesGetRawDataError(t *testing.T) {
+	// nil transaction makes GetRawData fail; previously the error was discarded
+	// and ledger.SignTx was called with a nil payload.
+	ctrl := NewController(nil, nil, nil, nil)
+
+	ctrl.hardwareSignTxForSending()
+
+	require.Error(t, ctrl.executionError, "expected executionError when GetRawData fails")
+	assert.ErrorContains(t, ctrl.executionError, "get raw data for ledger signing")
+	assert.ErrorContains(t, ctrl.executionError, "transaction is nil")
+	assert.Nil(t, ctrl.tx, "must not invent a transaction after GetRawData failure")
+}
+
+func TestGetRawData_NilTransaction(t *testing.T) {
+	ctrl := NewController(nil, nil, nil, nil)
+	raw, err := ctrl.GetRawData()
+	require.Error(t, err)
+	assert.Nil(t, raw)
+	assert.Contains(t, err.Error(), "transaction is nil")
+}
+
+func TestGetRawData_NilRawData(t *testing.T) {
+	// Non-nil tx with missing RawData used to marshal to empty bytes with no error,
+	// which hardwareSignTxForSending would then feed to ledger.SignTx.
+	ctrl := NewController(nil, nil, nil, &core.Transaction{})
+	raw, err := ctrl.GetRawData()
+	require.Error(t, err)
+	assert.Nil(t, raw)
+	assert.Contains(t, err.Error(), "transaction raw data is nil")
+}
+
+func TestGetRawData_EmptyRawData(t *testing.T) {
+	ctrl := NewController(nil, nil, nil, &core.Transaction{
+		RawData: &core.TransactionRaw{},
+	})
+	raw, err := ctrl.GetRawData()
+	require.Error(t, err)
+	assert.Nil(t, raw)
+	assert.Contains(t, err.Error(), "transaction raw data is empty")
+}
+
+func TestHardwareSignTxForSending_EmptyTransaction(t *testing.T) {
+	ctrl := NewController(nil, nil, nil, &core.Transaction{})
+
+	ctrl.hardwareSignTxForSending()
+
+	require.Error(t, ctrl.executionError, "expected executionError for empty transaction")
+	assert.ErrorContains(t, ctrl.executionError, "get raw data for ledger signing")
+	assert.ErrorContains(t, ctrl.executionError, "transaction raw data is nil")
+}
+
+func TestTransactionHash_NilTransaction(t *testing.T) {
+	ctrl := NewController(nil, nil, nil, nil)
+	hash, err := ctrl.TransactionHash()
+	require.Error(t, err)
+	assert.Empty(t, hash)
+	assert.Contains(t, err.Error(), "transaction is nil")
+}
+
 // ---------------------------------------------------------------------------
 // 3. sendSignedTx
 // ---------------------------------------------------------------------------
@@ -718,6 +791,22 @@ func TestTxConfirmation_FailedResultSetsResultError(t *testing.T) {
 	assert.Contains(t, ctrl.resultError.Error(), "out of energy")
 }
 
+func TestTxConfirmation_HashErrorIsWrapped(t *testing.T) {
+	// W50: previously the cause was replaced with a fixed string, losing the root error.
+	ctrl := NewController(nil, nil, nil, nil)
+	ctrl.Behavior.ConfirmationWaitTime = 1
+
+	ctrl.txConfirmation()
+
+	require.Error(t, ctrl.executionError, "expected executionError when hash computation fails")
+	assert.ErrorContains(t, ctrl.executionError, "could not get tx hash")
+	assert.ErrorContains(t, ctrl.executionError, "transaction is nil")
+	// %w must preserve the cause for errors.Unwrap / fmt %+v consumers.
+	cause := errors.Unwrap(ctrl.executionError)
+	require.Error(t, cause, "expected wrapped cause, got bare string error")
+	assert.ErrorContains(t, cause, "transaction is nil")
+}
+
 // ---------------------------------------------------------------------------
 // 6. GetResultError
 // ---------------------------------------------------------------------------

pkg/keystore/crypto_internal_test.go

@@ -371,6 +371,28 @@ func TestRecoverPubkey(t *testing.T) {
 		assert.Equal(t, expectedAddr, recovered)
 	})
 
+	t.Run("does not mutate caller's signature (W55)", func(t *testing.T) {
+		privKey, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader)
+		require.NoError(t, err)
+
+		hash := make([]byte, 32)
+		_, err = rand.Read(hash)
+		require.NoError(t, err)
+
+		sig, err := crypto.Sign(hash, privKey)
+		require.NoError(t, err)
+
+		// Ethereum-style V: force normalization path (v -= 27).
+		sig[64] += 27
+		original := make([]byte, 65)
+		copy(original, sig)
+
+		_, err = RecoverPubkey(hash, sig)
+		require.NoError(t, err)
+
+		assert.Equal(t, original, sig, "RecoverPubkey must not mutate the caller's signature slice")
+	})
+
 	t.Run("invalid signature bytes returns error", func(t *testing.T) {
 		hash := make([]byte, 32)
 		_, err := rand.Read(hash)

pkg/keystore/recover.go

@@ -12,15 +12,21 @@ import (
 )
 
 // RecoverPubkey recovers the TRON address from a message hash and its ECDSA signature.
+// The caller's signature slice is never modified; V-byte normalization
+// (Ethereum-style v >= 27), if needed, is applied to an internal copy.
 func RecoverPubkey(hash []byte, signature []byte) (address.Address, error) {
 	if len(signature) != 65 {
 		return nil, fmt.Errorf("invalid signature length: %d/65", len(signature))
 	}
-	if signature[64] >= 27 {
-		signature[64] -= 27
+	// Always copy so callers can re-verify, serialize, or broadcast the original
+	// signature without observing a mutated V byte.
+	sig := make([]byte, 65)
+	copy(sig, signature)
+	if sig[64] >= 27 {
+		sig[64] -= 27
 	}
 
-	sigPublicKey, err := crypto.Ecrecover(hash, signature)
+	sigPublicKey, err := crypto.Ecrecover(hash, sig)
 	if err != nil {
 		return nil, err
 	}