fix(core): prevent chain_id from serializing for requests

[Rjected]

Motivation

OpenEthereum seems to not have a chainId field in its TransactionRequest type.

Solution

This PR disables serialization for the chain_id, so we don't send it in RPC requests.

PR Checklist

• Added Tests
• Added Documentation
• Updated the changelog

Fixes #877

[mattsse]

hmm, this is a bit tricky, because I'm not sure what side-effects this could potentially cause, what does? since this is a client issue for openethereum
I believe in geth chainid is omitempty, so this approach seems fine. But not sure if there is some kind of check that chainids match so that tx intended for chain X are rejected on chain Y? so we'd lose that

alternatives:

• should we skip this instead based on the chainid?
• should we rather take the chain id if the client is OE, I believe we already have some calls that check the client first, definitely more effort, not ideal.

[Rjected]

Since chain_id was recently introduced to our request types, we shouldn't really be introducing any new issues by not serializing.

I'm not sure that alternative 1 (skip based on the chainid), or any solution that depends on only the chainid would work since OE will not accept any chainId field. Geth does have omitempty for the chainId, but it would be up to us to ensure that the chain_id field matches that of the client when running send_transaction, sign_transaction, etc. This would probably prevent user error.

This would not be necessary for send_raw_transaction, since that depends on RLP, which should already include the chainid. Geth does check the chainId, so if we were to do chainId checks there it would be in case other clients don't do these checks.

[mattsse]

thanks, sgtm!

[mattsse]

smol nit

[mattsse]

Suggested change

	#[serde(rename = "chainId")]
	#[serde(default, rename = "chainId")]

[Rjected]

added to both request and 1559 request

[tarrencev]

@Rjected I'm still running into this issue after updating foundry to use this PR

(code: -32602, message: Invalid params: unknown field `chainId`, expected one of `type`, `from`, `to`, `gasPrice`, `maxFeePerGas`, `gas`, `value`, `data`, `nonce`, `accessList`, `maxPriorityFeePerGas`., data: None)

It looks like it is still serialized into the txn \"chainId\":\"0x2a\"

[gakonst]

@Rjected maybe let's add a serialization test which ensures that neither txs have chainId in the serialized form?

[tarrencev]

@Rjected @gakonst Ah nevermind. I sanity checked forge paths and it was using an old path under .cargo rather than the one in .foundry