cmd, eth, rpc: fix some RPC issues with pending blocks

5 files changed, 107 insertions, 114 deletions

diff --git a/cmd/gethrpctest/main.go b/cmd/gethrpctest/main.go
index b4530ca51..8522258a9 100644
--- a/cmd/gethrpctest/main.go
+++ b/cmd/gethrpctest/main.go
@@ -52,6 +52,10 @@ var (
 func main() {
 	flag.Parse()
 
+	// Enable logging errors, we really do want to see those
+	glog.SetV(2)
+	glog.SetToStderr(true)
+
 	// Load the test suite to run the RPC against
 	tests, err := tests.LoadBlockTests(*testFile)
 	if err != nil {
diff --git a/eth/api.go b/eth/api.go
index bc7d17534..0fa855f15 100644
--- a/eth/api.go
+++ b/eth/api.go
@@ -53,19 +53,40 @@ const (
 	defaultGas      = uint64(90000)
 )
 
-// blockByNumber is a commonly used helper function which retrieves and returns the block for the given block number. It
-// returns nil when no block could be found.
+// blockByNumber is a commonly used helper function which retrieves and returns
+// the block for the given block number, capable of handling two special blocks:
+// rpc.LatestBlockNumber adn rpc.PendingBlockNumber. It returns nil when no block
+// could be found.
 func blockByNumber(m *miner.Miner, bc *core.BlockChain, blockNr rpc.BlockNumber) *types.Block {
+	// Pending block is only known by the miner
 	if blockNr == rpc.PendingBlockNumber {
 		return m.PendingBlock()
 	}
+	// Otherwise resolve and return the block
 	if blockNr == rpc.LatestBlockNumber {
 		return bc.CurrentBlock()
 	}
-
 	return bc.GetBlockByNumber(uint64(blockNr))
 }
 
+// stateAndBlockByNumber is a commonly used helper function which retrieves and
+// returns the state and containing block for the given block number, capable of
+// handling two special states: rpc.LatestBlockNumber adn rpc.PendingBlockNumber.
+// It returns nil when no block or state could be found.
+func stateAndBlockByNumber(m *miner.Miner, bc *core.BlockChain, blockNr rpc.BlockNumber, chainDb ethdb.Database) (*state.StateDB, *types.Block, error) {
+	// Pending state is only known by the miner
+	if blockNr == rpc.PendingBlockNumber {
+		return m.PendingState(), m.PendingBlock(), nil
+	}
+	// Otherwise resolve the block number and return its state
+	block := blockByNumber(m, bc, blockNr)
+	if block == nil {
+		return nil, nil, nil
+	}
+	stateDb, err := state.New(block.Root(), chainDb)
+	return stateDb, block, err
+}
+
 // PublicEthereumAPI provides an API to access Ethereum related information.
 // It offers only methods that operate on public data that is freely available to anyone.
 type PublicEthereumAPI struct {
@@ -395,16 +416,12 @@ func (s *PublicBlockChainAPI) BlockNumber() *big.Int {
 	return s.bc.CurrentHeader().Number
 }
 
-// GetBalance returns the amount of wei for the given address in the state of the given block number.
-// When block number equals rpc.LatestBlockNumber the current block is used.
+// GetBalance returns the amount of wei for the given address in the state of the
+// given block number. The rpc.LatestBlockNumber and rpc.PendingBlockNumber meta
+// block numbers are also allowed.
 func (s *PublicBlockChainAPI) GetBalance(address common.Address, blockNr rpc.BlockNumber) (*big.Int, error) {
-	block := blockByNumber(s.miner, s.bc, blockNr)
-	if block == nil {
-		return nil, nil
-	}
-
-	state, err := state.New(block.Root(), s.chainDb)
-	if err != nil {
+	state, _, err := stateAndBlockByNumber(s.miner, s.bc, blockNr, s.chainDb)
+	if state == nil || err != nil {
 		return nil, err
 	}
 	return state.GetBalance(address), nil
@@ -414,7 +431,14 @@ func (s *PublicBlockChainAPI) GetBalance(address common.Address, blockNr rpc.Blo
 // transactions in the block are returned in full detail, otherwise only the transaction hash is returned.
 func (s *PublicBlockChainAPI) GetBlockByNumber(blockNr rpc.BlockNumber, fullTx bool) (map[string]interface{}, error) {
 	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
-		return s.rpcOutputBlock(block, true, fullTx)
+		response, err := s.rpcOutputBlock(block, true, fullTx)
+		if err == nil && blockNr == rpc.PendingBlockNumber {
+			// Pending blocks need to nil out a few fields
+			for _, field := range []string{"hash", "nonce", "logsBloom", "miner"} {
+				response[field] = nil
+			}
+		}
+		return response, err
 	}
 	return nil, nil
 }
@@ -431,10 +455,6 @@ func (s *PublicBlockChainAPI) GetBlockByHash(blockHash common.Hash, fullTx bool)
 // GetUncleByBlockNumberAndIndex returns the uncle block for the given block hash and index. When fullTx is true
 // all transactions in the block are returned in full detail, otherwise only the transaction hash is returned.
 func (s *PublicBlockChainAPI) GetUncleByBlockNumberAndIndex(blockNr rpc.BlockNumber, index rpc.HexNumber) (map[string]interface{}, error) {
-	if blockNr == rpc.PendingBlockNumber {
-		return nil, nil
-	}
-
 	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
 		uncles := block.Uncles()
 		if index.Int() < 0 || index.Int() >= len(uncles) {
@@ -464,10 +484,6 @@ func (s *PublicBlockChainAPI) GetUncleByBlockHashAndIndex(blockHash common.Hash,
 
 // GetUncleCountByBlockNumber returns number of uncles in the block for the given block number
 func (s *PublicBlockChainAPI) GetUncleCountByBlockNumber(blockNr rpc.BlockNumber) *rpc.HexNumber {
-	if blockNr == rpc.PendingBlockNumber {
-		return rpc.NewHexNumber(0)
-	}
-
 	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
 		return rpc.NewHexNumber(len(block.Uncles()))
 	}
@@ -508,38 +524,26 @@ func (s *PublicBlockChainAPI) NewBlocks(args NewBlocksArgs) (rpc.Subscription, e
 
 // GetCode returns the code stored at the given address in the state for the given block number.
 func (s *PublicBlockChainAPI) GetCode(address common.Address, blockNr rpc.BlockNumber) (string, error) {
-	return s.GetData(address, blockNr)
-}
-
-// GetData returns the data stored at the given address in the state for the given block number.
-func (s *PublicBlockChainAPI) GetData(address common.Address, blockNr rpc.BlockNumber) (string, error) {
-	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
-		state, err := state.New(block.Root(), s.chainDb)
-		if err != nil {
-			return "", err
-		}
-		res := state.GetCode(address)
-		if len(res) == 0 { // backwards compatibility
-			return "0x", nil
-		}
-		return common.ToHex(res), nil
+	state, _, err := stateAndBlockByNumber(s.miner, s.bc, blockNr, s.chainDb)
+	if state == nil || err != nil {
+		return "", err
 	}
-
-	return "0x", nil
+	res := state.GetCode(address)
+	if len(res) == 0 { // backwards compatibility
+		return "0x", nil
+	}
+	return common.ToHex(res), nil
 }
 
-// GetStorageAt returns the storage from the state at the given address, key and block number.
+// GetStorageAt returns the storage from the state at the given address, key and
+// block number. The rpc.LatestBlockNumber and rpc.PendingBlockNumber meta block
+// numbers are also allowed.
 func (s *PublicBlockChainAPI) GetStorageAt(address common.Address, key string, blockNr rpc.BlockNumber) (string, error) {
-	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
-		state, err := state.New(block.Root(), s.chainDb)
-		if err != nil {
-			return "", err
-		}
-
-		return state.GetState(address, common.HexToHash(key)).Hex(), nil
+	state, _, err := stateAndBlockByNumber(s.miner, s.bc, blockNr, s.chainDb)
+	if state == nil || err != nil {
+		return "0x", err
 	}
-
-	return "0x", nil
+	return state.GetState(address, common.HexToHash(key)).Hex(), nil
 }
 
 // callmsg is the message type used for call transations.
@@ -570,55 +574,51 @@ type CallArgs struct {
 }
 
 func (s *PublicBlockChainAPI) doCall(args CallArgs, blockNr rpc.BlockNumber) (string, *big.Int, error) {
-	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
-		stateDb, err := state.New(block.Root(), s.chainDb)
-		if err != nil {
-			return "0x", nil, err
-		}
-
-		stateDb = stateDb.Copy()
-		var from *state.StateObject
-		if args.From == (common.Address{}) {
-			accounts, err := s.am.Accounts()
-			if err != nil || len(accounts) == 0 {
-				from = stateDb.GetOrNewStateObject(common.Address{})
-			} else {
-				from = stateDb.GetOrNewStateObject(accounts[0].Address)
-			}
+	// Fetch the state associated with the block number
+	stateDb, block, err := stateAndBlockByNumber(s.miner, s.bc, blockNr, s.chainDb)
+	if stateDb == nil || err != nil {
+		return "0x", nil, err
+	}
+	stateDb = stateDb.Copy()
+
+	// Retrieve the account state object to interact with
+	var from *state.StateObject
+	if args.From == (common.Address{}) {
+		accounts, err := s.am.Accounts()
+		if err != nil || len(accounts) == 0 {
+			from = stateDb.GetOrNewStateObject(common.Address{})
 		} else {
-			from = stateDb.GetOrNewStateObject(args.From)
-		}
-
-		from.SetBalance(common.MaxBig)
-
-		msg := callmsg{
-			from:     from,
-			to:       &args.To,
-			gas:      args.Gas.BigInt(),
-			gasPrice: args.GasPrice.BigInt(),
-			value:    args.Value.BigInt(),
-			data:     common.FromHex(args.Data),
-		}
-
-		if msg.gas.Cmp(common.Big0) == 0 {
-			msg.gas = big.NewInt(50000000)
-		}
-
-		if msg.gasPrice.Cmp(common.Big0) == 0 {
-			msg.gasPrice = new(big.Int).Mul(big.NewInt(50), common.Shannon)
+			from = stateDb.GetOrNewStateObject(accounts[0].Address)
 		}
+	} else {
+		from = stateDb.GetOrNewStateObject(args.From)
+	}
+	from.SetBalance(common.MaxBig)
 
-		header := s.bc.CurrentBlock().Header()
-		vmenv := core.NewEnv(stateDb, s.bc, msg, header)
-		gp := new(core.GasPool).AddGas(common.MaxBig)
-		res, gas, err := core.ApplyMessage(vmenv, msg, gp)
-		if len(res) == 0 { // backwards compatability
-			return "0x", gas, err
-		}
-		return common.ToHex(res), gas, err
+	// Assemble the CALL invocation
+	msg := callmsg{
+		from:     from,
+		to:       &args.To,
+		gas:      args.Gas.BigInt(),
+		gasPrice: args.GasPrice.BigInt(),
+		value:    args.Value.BigInt(),
+		data:     common.FromHex(args.Data),
 	}
+	if msg.gas.Cmp(common.Big0) == 0 {
+		msg.gas = big.NewInt(50000000)
+	}
+	if msg.gasPrice.Cmp(common.Big0) == 0 {
+		msg.gasPrice = new(big.Int).Mul(big.NewInt(50), common.Shannon)
+	}
+	// Execute the call and return
+	vmenv := core.NewEnv(stateDb, s.bc, msg, block.Header())
+	gp := new(core.GasPool).AddGas(common.MaxBig)
 
-	return "0x", common.Big0, nil
+	res, gas, err := core.ApplyMessage(vmenv, msg, gp)
+	if len(res) == 0 { // backwards compatibility
+		return "0x", gas, err
+	}
+	return common.ToHex(res), gas, err
 }
 
 // Call executes the given transaction on the state for the given block number.
@@ -802,14 +802,9 @@ func getTransaction(chainDb ethdb.Database, txPool *core.TxPool, txHash common.H
 
 // GetBlockTransactionCountByNumber returns the number of transactions in the block with the given block number.
 func (s *PublicTransactionPoolAPI) GetBlockTransactionCountByNumber(blockNr rpc.BlockNumber) *rpc.HexNumber {
-	if blockNr == rpc.PendingBlockNumber {
-		return rpc.NewHexNumber(0)
-	}
-
 	if block := blockByNumber(s.miner, s.bc, blockNr); block != nil {
 		return rpc.NewHexNumber(len(block.Transactions()))
 	}
-
 	return nil
 }
 
@@ -839,13 +834,8 @@ func (s *PublicTransactionPoolAPI) GetTransactionByBlockHashAndIndex(blockHash c
 
 // GetTransactionCount returns the number of transactions the given address has sent for the given block number
 func (s *PublicTransactionPoolAPI) GetTransactionCount(address common.Address, blockNr rpc.BlockNumber) (*rpc.HexNumber, error) {
-	block := blockByNumber(s.miner, s.bc, blockNr)
-	if block == nil {
-		return nil, nil
-	}
-
-	state, err := state.New(block.Root(), s.chainDb)
-	if err != nil {
+	state, _, err := stateAndBlockByNumber(s.miner, s.bc, blockNr, s.chainDb)
+	if state == nil || err != nil {
 		return nil, err
 	}
 	return rpc.NewHexNumber(state.GetNonce(address)), nil
diff --git a/eth/filters/api.go b/eth/filters/api.go
index f2b0ed32f..aa4c305a6 100644
--- a/eth/filters/api.go
+++ b/eth/filters/api.go
@@ -239,13 +239,13 @@ func (args *NewFilterArgs) UnmarshalJSON(data []byte) error {
 		return err
 	}
 
-	if raw.From == nil {
+	if raw.From == nil || raw.From.Int64() < 0 {
 		args.FromBlock = rpc.LatestBlockNumber
 	} else {
 		args.FromBlock = *raw.From
 	}
 
-	if raw.ToBlock == nil {
+	if raw.ToBlock == nil || raw.ToBlock.Int64() < 0 {
 		args.ToBlock = rpc.LatestBlockNumber
 	} else {
 		args.ToBlock = *raw.ToBlock
diff --git a/rpc/server.go b/rpc/server.go
index 0b93a4e64..5b88d843a 100644
--- a/rpc/server.go
+++ b/rpc/server.go
@@ -332,7 +332,6 @@ func (s *Server) handle(ctx context.Context, codec ServerCodec, req *serverReque
 			return res
 		}
 	}
-
 	return codec.CreateResponse(req.id, reply[0].Interface())
 }
 
@@ -344,7 +343,6 @@ func (s *Server) exec(ctx context.Context, codec ServerCodec, req *serverRequest
 	} else {
 		response = s.handle(ctx, codec, req)
 	}
-
 	if err := codec.Write(response); err != nil {
 		glog.V(logger.Error).Infof("%v\n", err)
 		codec.Close()
diff --git a/rpc/types.go b/rpc/types.go
index 02295a022..f268d84db 100644
--- a/rpc/types.go
+++ b/rpc/types.go
@@ -174,12 +174,14 @@ type HexNumber big.Int
 // NewHexNumber creates a new hex number instance which will serialize the given val with `%#x` on marshal.
 func NewHexNumber(val interface{}) *HexNumber {
 	if val == nil {
-		return nil
+		return nil // note, this doesn't catch nil pointers, only passing nil directly!
 	}
 
-	if v, ok := val.(*big.Int); ok && v != nil {
-		hn := new(big.Int).Set(v)
-		return (*HexNumber)(hn)
+	if v, ok := val.(*big.Int); ok {
+		if v != nil {
+			return (*HexNumber)(new(big.Int).Set(v))
+		}
+		return nil
 	}
 
 	rval := reflect.ValueOf(val)
@@ -303,10 +305,9 @@ const (
 )
 
 // UnmarshalJSON parses the given JSON fragement into a BlockNumber. It supports:
-// - "latest" or "earliest" as string arguments
+// - "latest", "earliest" or "pending" as string arguments
 // - the block number
 // Returned errors:
-// - an unsupported error when "pending" is specified (not yet implemented)
 // - an invalid block number error when the given argument isn't a known strings
 // - an out of range error when the given block number is either too little or too large
 func (bn *BlockNumber) UnmarshalJSON(data []byte) error {