From 302bc9f7fbaabaa1f53b5b04b067df0192476e24 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Mon, 25 Sep 2017 11:10:37 +0200
Subject: [PATCH] ethclient: require inclusion block for TransactionSender

---
 ethclient/ethclient.go | 32 +++++++++++++++++++-------------
 ethclient/signer.go    | 13 +++++++------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/ethclient/ethclient.go b/ethclient/ethclient.go
index 5747fc5df1..5a2720c603 100644
--- a/ethclient/ethclient.go
+++ b/ethclient/ethclient.go
@@ -20,6 +20,7 @@ package ethclient
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -166,6 +167,7 @@ type rpcTransaction struct {
 
 type txExtraInfo struct {
 	BlockNumber *string
+	BlockHash   common.Hash
 	From        common.Address
 }
 
@@ -187,29 +189,33 @@ func (ec *Client) TransactionByHash(ctx context.Context, hash common.Hash) (tx *
 	} else if _, r, _ := json.tx.RawSignatureValues(); r == nil {
 		return nil, false, fmt.Errorf("server returned transaction without signature")
 	}
-	setSenderFromServer(json.tx, json.From)
+	setSenderFromServer(json.tx, json.From, json.BlockHash)
 	return json.tx, json.BlockNumber == nil, nil
 }
 
 // TransactionSender returns the sender address of the given transaction. The transaction
-// must be known to the remote node. The sender is the one derived by the protocol at the
-// time of inclusion.
+// must be known to the remote node and included in the blockchain. The sender is the one
+// derived by the protocol at the time of inclusion.
 //
 // There is a fast-path for transactions retrieved by TransactionByHash and
 // TransactionInBlock. Getting their sender address can be done without an RPC interaction.
-func (ec *Client) TransactionSender(ctx context.Context, tx *types.Transaction) (common.Address, error) {
+func (ec *Client) TransactionSender(ctx context.Context, tx *types.Transaction, block common.Hash, index uint) (common.Address, error) {
 	// Try to load the address from the cache.
-	sender, err := types.Sender((*senderFromServer)(nil), tx)
+	sender, err := types.Sender(&senderFromServer{blockhash: block}, tx)
 	if err == nil {
 		return sender, nil
 	}
-	// TODO It'd better to use GetTransactionByBlockHashAndIndex because it works with the
-	// light client. This will be even more important with EIP 208 because there can be
-	// multiple inclusions of the same tx. The downside is that users would need to supply
-	// the inclusion block.
-	var meta struct{ From common.Address }
-	err = ec.c.CallContext(ctx, &meta, "eth_getTransactionByHash", tx.Hash())
-	return meta.From, err
+	var meta struct {
+		Hash common.Hash
+		From common.Address
+	}
+	if err = ec.c.CallContext(ctx, &meta, "eth_getTransactionByBlockHashAndIndex", block, hexutil.Uint64(index)); err != nil {
+		return common.Address{}, err
+	}
+	if meta.Hash == (common.Hash{}) || meta.Hash != tx.Hash() {
+		return common.Address{}, errors.New("wrong inclusion block/index")
+	}
+	return meta.From, nil
 }
 
 // TransactionCount returns the total number of transactions in the given block.
@@ -230,7 +236,7 @@ func (ec *Client) TransactionInBlock(ctx context.Context, blockHash common.Hash,
 			return nil, fmt.Errorf("server returned transaction without signature")
 		}
 	}
-	setSenderFromServer(json.tx, json.From)
+	setSenderFromServer(json.tx, json.From, json.BlockHash)
 	return json.tx, err
 }
 
diff --git a/ethclient/signer.go b/ethclient/signer.go
index 8c16bc8b6b..74a93f1e2f 100644
--- a/ethclient/signer.go
+++ b/ethclient/signer.go
@@ -28,23 +28,24 @@ import (
 // server. It is stored in the transaction's sender address cache to avoid an additional
 // request in TransactionSender.
 type senderFromServer struct {
-	addr common.Address
+	addr      common.Address
+	blockhash common.Hash
 }
 
 var errNotCached = errors.New("sender not cached")
 
-func setSenderFromServer(tx *types.Transaction, addr common.Address) {
+func setSenderFromServer(tx *types.Transaction, addr common.Address, block common.Hash) {
 	// Use types.Sender for side-effect to store our signer into the cache.
-	types.Sender(&senderFromServer{addr}, tx)
+	types.Sender(&senderFromServer{addr, block}, tx)
 }
 
 func (s *senderFromServer) Equal(other types.Signer) bool {
-	_, ok := other.(*senderFromServer)
-	return ok
+	os, ok := other.(*senderFromServer)
+	return ok && os.blockhash == s.blockhash
 }
 
 func (s *senderFromServer) Sender(tx *types.Transaction) (common.Address, error) {
-	if s == nil {
+	if s.blockhash == (common.Hash{}) {
 		return common.Address{}, errNotCached
 	}
 	return s.addr, nil