cleanup: Remove OVM signer #278
Conversation
| // representation, with the given location metadata set (if available). | ||
| func newRPCTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber uint64, index uint64) *RPCTransaction { | ||
| var signer types.Signer = types.NewOVMSigner(tx.ChainId()) | ||
| var signer types.Signer = types.FrontierSigner{} |
There was a problem hiding this comment.
This is reintroducing the types.FrontierSigner{} that was removed in a recent commit
| case config.IsHomestead(blockNumber): | ||
| signer = HomesteadSigner{} | ||
| default: | ||
| signer = FrontierSigner{} |
There was a problem hiding this comment.
we never want to return anything besides EIP155Signer. This would result in a diff but would give extra assurance that we never use the wrong signer
There was a problem hiding this comment.
What impact would using the wrong signer have?
There was a problem hiding this comment.
Allowing transactions that are not protected with a chainid. All transactions must be protected with a chainid
There was a problem hiding this comment.
Transactions w/o chain ID should fail inside the OVM_ECDSAContractAccount.
|
@tynes we're based on Geth 1.9.10. The non-EIP155 signers are allowed there. They were only removed in Geth 1.10. Once we rebase to it, they will also be removed from our codebase, but for now I believe we should stick to minimizing the diff from 1.9.10. Also, the chainId is checked here in the OVM ECDSA Contract Account. |
|
This is a non-backwards compatible change meaning that historical sync depends on this - the RPC endpoint go-ethereum/internal/ethapi/api.go Line 1661 in b6ad0b3 This can only be removed if:
We will no longer support the EthSign based transactions at some point, meaning we will need to sunset the functionality We also have a hard requirement that the sequencer must reject transactions sent directly to it without a chainid |
Description
We're not using this code anymore, should be able to safely remove it.
Contributing Agreement