ethclient, ethereum: add NotFound, split transactions out of ChainReader

ethclient now returns ethereum.NotFound if the server returns null and
no error while accessing blockchain data.

The light client cannot provide arbitrary transactions. The change to
split transaction access into its own interface emphasizes that
transactions should not be relied on and recommends use of logs.

4 files changed, 67 insertions, 23 deletions

diff --git a/ethclient/ethclient.go b/ethclient/ethclient.go
index b23e9baa3..4daebda92 100644
--- a/ethclient/ethclient.go
+++ b/ethclient/ethclient.go
@@ -81,6 +81,8 @@ func (ec *Client) getBlock(ctx context.Context, method string, args ...interface
 	err := ec.c.CallContext(ctx, &raw, method, args...)
 	if err != nil {
 		return nil, err
+	} else if len(raw) == 0 {
+		return nil, ethereum.NotFound
 	}
 	// Decode header and transactions.
 	var head *types.Header
@@ -135,6 +137,9 @@ func (ec *Client) getBlock(ctx context.Context, method string, args ...interface
 func (ec *Client) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) {
 	var head *types.Header
 	err := ec.c.CallContext(ctx, &head, "eth_getBlockByHash", hash, false)
+	if err == nil && head == nil {
+		err = ethereum.NotFound
+	}
 	return head, err
 }
 
@@ -143,19 +148,31 @@ func (ec *Client) HeaderByHash(ctx context.Context, hash common.Hash) (*types.He
 func (ec *Client) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) {
 	var head *types.Header
 	err := ec.c.CallContext(ctx, &head, "eth_getBlockByNumber", toBlockNumArg(number), false)
+	if err == nil && head == nil {
+		err = ethereum.NotFound
+	}
 	return head, err
 }
 
 // TransactionByHash returns the transaction with the given hash.
-func (ec *Client) TransactionByHash(ctx context.Context, hash common.Hash) (*types.Transaction, error) {
-	var tx *types.Transaction
-	err := ec.c.CallContext(ctx, &tx, "eth_getTransactionByHash", hash)
-	if err == nil {
-		if _, r, _ := tx.RawSignatureValues(); r == nil {
-			return nil, fmt.Errorf("server returned transaction without signature")
-		}
+func (ec *Client) TransactionByHash(ctx context.Context, hash common.Hash) (tx *types.Transaction, isPending bool, err error) {
+	var raw json.RawMessage
+	err = ec.c.CallContext(ctx, &raw, "eth_getTransactionByHash", hash)
+	if err != nil {
+		return nil, false, err
+	} else if len(raw) == 0 {
+		return nil, false, ethereum.NotFound
 	}
-	return tx, err
+	if err := json.Unmarshal(raw, tx); err != nil {
+		return nil, false, err
+	} else if _, r, _ := tx.RawSignatureValues(); r == nil {
+		return nil, false, fmt.Errorf("server returned transaction without signature")
+	}
+	var block struct{ BlockHash *common.Hash }
+	if err := json.Unmarshal(raw, &block); err != nil {
+		return nil, false, err
+	}
+	return tx, block.BlockHash == nil, nil
 }
 
 // TransactionCount returns the total number of transactions in the given block.
@@ -170,11 +187,9 @@ func (ec *Client) TransactionInBlock(ctx context.Context, blockHash common.Hash,
 	var tx *types.Transaction
 	err := ec.c.CallContext(ctx, &tx, "eth_getTransactionByBlockHashAndIndex", blockHash, index)
 	if err == nil {
-		var signer types.Signer = types.HomesteadSigner{}
-		if tx.Protected() {
-			signer = types.NewEIP155Signer(tx.ChainId())
-		}
-		if _, r, _ := types.SignatureValues(signer, tx); r == nil {
+		if tx == nil {
+			return nil, ethereum.NotFound
+		} else if _, r, _ := tx.RawSignatureValues(); r == nil {
 			return nil, fmt.Errorf("server returned transaction without signature")
 		}
 	}
@@ -186,8 +201,12 @@ func (ec *Client) TransactionInBlock(ctx context.Context, blockHash common.Hash,
 func (ec *Client) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
 	var r *types.Receipt
 	err := ec.c.CallContext(ctx, &r, "eth_getTransactionReceipt", txHash)
-	if err == nil && r != nil && len(r.PostState) == 0 {
-		return nil, fmt.Errorf("server returned receipt without post state")
+	if err == nil {
+		if r == nil {
+			return nil, ethereum.NotFound
+		} else if len(r.PostState) == 0 {
+			return nil, fmt.Errorf("server returned receipt without post state")
+		}
 	}
 	return r, err
 }
diff --git a/ethclient/ethclient_test.go b/ethclient/ethclient_test.go
index 102c0d3b2..178eb2be9 100644
--- a/ethclient/ethclient_test.go
+++ b/ethclient/ethclient_test.go
@@ -21,9 +21,9 @@ import "github.com/ethereum/go-ethereum"
 // Verify that Client implements the ethereum interfaces.
 var (
 	_ = ethereum.ChainReader(&Client{})
+	_ = ethereum.TransactionReader(&Client{})
 	_ = ethereum.ChainStateReader(&Client{})
 	_ = ethereum.ChainSyncReader(&Client{})
-	_ = ethereum.ChainHeadEventer(&Client{})
 	_ = ethereum.ContractCaller(&Client{})
 	_ = ethereum.GasEstimator(&Client{})
 	_ = ethereum.GasPricer(&Client{})
diff --git a/interfaces.go b/interfaces.go
index aab0e2029..5e38a9054 100644
--- a/interfaces.go
+++ b/interfaces.go
@@ -18,6 +18,7 @@
 package ethereum
 
 import (
+	"errors"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -26,6 +27,9 @@ import (
 	"golang.org/x/net/context"
 )
 
+// NotFound is returned by API methods if the requested item does not exist.
+var NotFound = errors.New("not found")
+
 // TODO: move subscription to package event
 
 // Subscription represents an event subscription where events are
@@ -46,6 +50,8 @@ type Subscription interface {
 // blockchain fork that was previously downloaded and processed by the node. The block
 // number argument can be nil to select the latest canonical block. Reading block headers
 // should be preferred over full blocks whenever possible.
+//
+// The returned error is NotFound if the requested item does not exist.
 type ChainReader interface {
 	BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error)
 	BlockByNumber(ctx context.Context, number *big.Int) (*types.Block, error)
@@ -53,7 +59,30 @@ type ChainReader interface {
 	HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error)
 	TransactionCount(ctx context.Context, blockHash common.Hash) (uint, error)
 	TransactionInBlock(ctx context.Context, blockHash common.Hash, index uint) (*types.Transaction, error)
-	TransactionByHash(ctx context.Context, txHash common.Hash) (*types.Transaction, error)
+
+	// This method subscribes to notifications about changes of the head block of
+	// the canonical chain.
+	SubscribeNewHead(ctx context.Context, ch chan<- *types.Header) (Subscription, error)
+}
+
+// TransactionReader provides access to past transactions and their receipts.
+// Implementations may impose arbitrary restrictions on the transactions and receipts that
+// can be retrieved. Historic transactions may not be available.
+//
+// Avoid relying on this interface if possible. Contract logs (through the LogFilterer
+// interface) are more reliable and usually safer in the presence of chain
+// reorganisations.
+//
+// The returned error is NotFound if the requested item does not exist.
+type TransactionReader interface {
+	// TransactionByHash checks the pool of pending transactions in addition to the
+	// blockchain. The isPending return value indicates whether the transaction has been
+	// mined yet. Note that the transaction may not be part of the canonical chain even if
+	// it's not pending.
+	TransactionByHash(ctx context.Context, txHash common.Hash) (tx *types.Transaction, isPending bool, err error)
+	// TransactionReceipt returns the receipt of a mined transaction. Note that the
+	// transaction may not be included in the current canonical chain even if a receipt
+	// exists.
 	TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error)
 }
 
@@ -83,11 +112,6 @@ type ChainSyncReader interface {
 	SyncProgress(ctx context.Context) (*SyncProgress, error)
 }
 
-// A ChainHeadEventer returns notifications whenever the canonical head block is updated.
-type ChainHeadEventer interface {
-	SubscribeNewHead(ctx context.Context, ch chan<- *types.Header) (Subscription, error)
-}
-
 // CallMsg contains parameters for contract calls.
 type CallMsg struct {
 	From     common.Address  // the sender of the 'transaction'
diff --git a/mobile/ethclient.go b/mobile/ethclient.go
index 668d65e32..a60fc2fa5 100644
--- a/mobile/ethclient.go
+++ b/mobile/ethclient.go
@@ -73,7 +73,8 @@ func (ec *EthereumClient) GetHeaderByNumber(ctx *Context, number int64) (*Header
 
 // GetTransactionByHash returns the transaction with the given hash.
 func (ec *EthereumClient) GetTransactionByHash(ctx *Context, hash *Hash) (*Transaction, error) {
-	tx, err := ec.client.TransactionByHash(ctx.context, hash.hash)
+	// TODO(karalabe): handle isPending
+	tx, _, err := ec.client.TransactionByHash(ctx.context, hash.hash)
 	return &Transaction{tx}, err
 }