fix: non-eip-155 tx panic when get signer

[yihuang]

Closes: #280

Description

Closes: #XXXX

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• tackled an existing issue or discussed with a team member
• left instructions on how to review the changes
• targeted the main branch

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

• added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• confirmed all author checklist items have been addressed
• confirmed that this PR does not change production code
• reviewed content
• tested instructions (if applicable)
• confirmed all CI checks have passed

[mmsqe]

seems there's chain id check in legacy tx

[yihuang]

seems there's chain id check in legacy tx

treat 0 specially in get signer now, not ideal, but avoid changing too many code.

[yihuang]

I think passing tx.ChainID to LatestSignerForChainID is not safe, we should pass the current network chainID to it.
WDYT @vladjdk @zsystm @cloudgray

[vladjdk]

@yihuang What makes you say it's not safe? Seems like LatestSignerForChainID handles nils, and if a chain doesn't allow unprotected transactions, then the ante will fail out anyways:

evm/ante/evm/05_signature_verification.go

Lines 73 to 84 in 32be4de

	if !allowUnprotectedTxs {
	if !ethTx.Protected() {
	return errorsmod.Wrapf(
	errortypes.ErrNotSupported,
	"rejected unprotected ethereum transaction; please sign your transaction according to EIP-155 to protect it against replay-attacks")
	}
	if ethTx.ChainId().Uint64() != ethCfg.ChainID.Uint64() {
	return errorsmod.Wrapf(
	errortypes.ErrInvalidChainID,
	"rejected ethereum transaction with incorrect chain-id; expected %d, got %d", ethCfg.ChainID, ethTx.ChainId())
	}
	}

[yihuang]

@yihuang What makes you say it's not safe? Seems like LatestSignerForChainID handles nils, and if a chain doesn't allow unprotected transactions, then the ante will fail out anyways:

evm/ante/evm/05_signature_verification.go

Lines 73 to 84 in 32be4de

	if !allowUnprotectedTxs {
	if !ethTx.Protected() {
	return errorsmod.Wrapf(
	errortypes.ErrNotSupported,
	"rejected unprotected ethereum transaction; please sign your transaction according to EIP-155 to protect it against replay-attacks")
	}
	if ethTx.ChainId().Uint64() != ethCfg.ChainID.Uint64() {
	return errorsmod.Wrapf(
	errortypes.ErrInvalidChainID,
	"rejected ethereum transaction with incorrect chain-id; expected %d, got %d", ethCfg.ChainID, ethTx.ChainId())
	}
	}

I mean, normally the signer will recover sender from signature and check tx.chainId==network.chainId, assuing we pass network.chainId as parameter, but here we pass tx.chainId as parameter, so the second part of the verification is always true. it may not cause an immediate issue, but it's not how the API is expected to be called I think.

[vladjdk]

Ah, I think you're right. Feel free to add that change in this PR or open a new one if that makes more sense. I also don't think it'll cause any issues now, though.

[yihuang]

Ah, I think you're right. Feel free to add that change in this PR or open a new one if that makes more sense. I also don't think it'll cause any issues now, though.

cyclic import if try to import "github.com/cosmos/evm/x/vm/types",
but to change the function signature, need to change cosmos-sdk,
so it's a little bit tricky here.

[zsystm]

LGTM.