core/types: more easily extensible tx signing

[piersy]

Having just a single return point in methods that build signers makes it easier for forks of go ethereum to add their own signers that delegate to the underlying signer chain, since they can intercept the signer chain at a single point.

At celo we are currently transitioning to be an L2 based on the optimism codebase and we have a number of additional transaction types that we have extended vanilla ethereum with, these transaction types require custom signing therefore requiring us to modify the signer creation functions.

In order to lessen the burden of being a fork of op-geth and consequently go-ethereum we are trying to ensure that our changes are minimally invasive to the codebase, we've partially achieved that for transaction signing by having an overlay signer that handles our added transactions and delegates to the existing signer chain for all other cases.

In the case of MakeSigner(see below) we are able to cleanly add this since we can delegate to the signer variable, however for LatestSigner and LatestSignerForChainId this is not possible due to the multiple return points.

// MakeSigner returns a Signer based on the given chain config and block number.
func MakeSigner(config *params.ChainConfig, blockNumber *big.Int, blockTime uint64) Signer {
	var signer Signer
	switch {
	case config.IsCancun(blockNumber, blockTime) && !config.IsOptimism():
		signer = NewCancunSigner(config.ChainID)
	case config.IsLondon(blockNumber):
		signer = NewLondonSigner(config.ChainID)
	case config.IsBerlin(blockNumber):
		signer = NewEIP2930Signer(config.ChainID)
	case config.IsEIP155(blockNumber):
		signer = NewEIP155Signer(config.ChainID)
	case config.IsHomestead(blockNumber):
		signer = HomesteadSigner{}
	default:
		signer = FrontierSigner{}
	}

	// Apply the celo overlay signer, if no celo forks have been enabled then the given signer is returned.
	signer = makeCeloSigner(config, blockTime, signer)

	return signer
}

Having multiple return points requires us to handle potentially every return point inside the switch case or if ladder, which further introduces conflicts when forks with new signing are introduced.

I'm aware that this change doesn't bring any direct benefit for the go-ethereum codebase, but also I don't think it is further complicating the codebase and should help to simplify the lives of fork maintainers.

[holiman]

The way you did this, you always instantiate the default, then instantiate the non-default. IMO let's instantiate only once, e.g. by doing

	var signer Signer
	if config.ChainID != nil {
		switch {
		case config.CancunTime != nil:
			signer = NewCancunSigner(config.ChainID)
		case config.LondonBlock != nil:
			signer = NewLondonSigner(config.ChainID)
		case config.BerlinBlock != nil:
			signer = NewEIP2930Signer(config.ChainID)
		case config.EIP155Block != nil:
			signer = NewEIP155Signer(config.ChainID)
		default:
			signer = HomesteadSigner{}
		}
	} else {
		signer = HomesteadSigner{}
	}
	return signer

[piersy]

Hi @holiman, good point. I felt it was a little cleaner without the extra else but I wasn't considering instantiations at the time. Will update.

[piersy]

Hi @holiman, just letting you know I have made the changes you suggested.

[holiman]

LGTM