internal/ethapi: fix eth_chainId method

[piersy]

Although the ChainID is represented as a *big.Int in the ChainConfig, it
was being marshaled as a uint64 by the PublicEthereumAPI. This leads to
discrepancies between the config and calls to the API when the ChainID
is bigger than math.MaxUint64.

[rjl493456442]

Thanks for catching it. Apparently, we define two ChainID RPC calls, and even worse, they are all in the same namespace eth.

Here: https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L533
And here: https://github.com/ethereum/go-ethereum/blob/master/eth/api.go#L70

I think we can just remove the API in internal/ethapi/api.go and change the return to hexutil.Big in the eth/api

[fjl]

I would prefer to keep the one in internal/ethapi and delete the implementation in package eth.

[piersy]

Thanks for catching it. Apparently, we define two ChainID RPC calls, and even worse, they are all in the same namespace eth.

Here: https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L533
And here: https://github.com/ethereum/go-ethereum/blob/master/eth/api.go#L70

I think we can just remove the API in internal/ethapi/api.go and change the return to hexutil.Big in the eth/api

Yeah, this did confuse me

[piersy]

@Fjf @rjl493456442 Just checking in to see if you were expecting any action on my part? I was thinking I'd wait for agreement between you two before taking any further action.

[karalabe]

Ok, I agree with @fjl. The one in internal is neeed because it's used by both eth and les. The one in eth apparently overrides it, but it's redundant. Still, the implementation of the one in eth is the more correct one as it handles a chain progress issue.

Could you move the code from eth into the internal one and delete the eth version? Thanks

[holiman]

@karalabe do we want to add something like this?

diff --git a/rpc/service.go b/rpc/service.go
index bef891ea11..e30f8d7382 100644
--- a/rpc/service.go
+++ b/rpc/service.go
@@ -26,6 +26,7 @@ import (
 	"sync"
 	"unicode"
 
+	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -82,11 +83,17 @@ func (r *serviceRegistry) registerName(name string, rcvr interface{}) error {
 		}
 		r.services[name] = svc
 	}
-	for name, cb := range callbacks {
+	for methodName, cb := range callbacks {
 		if cb.isSubscribe {
-			svc.subscriptions[name] = cb
+			if _, exist := svc.subscriptions[methodName]; exist {
+				utils.Fatalf("API double-register of %v.%v\n", name, methodName)
+			}
+			svc.subscriptions[methodName] = cb
 		} else {
-			svc.callbacks[name] = cb
+			if _, exist := svc.callbacks[methodName]; exist {
+				utils.Fatalf("Double-register of %v.%v\n", name, methodName)
+			}
+			svc.callbacks[methodName] = cb
 		}
 	}
 	return nil

[piersy]

Could you move the code from eth into the internal one and delete the eth version? Thanks

Hi @karalabe I've made this change

[karalabe]

SGTM