Ensure TransactionArgs works with cip-64 and cip-66

[hbandura]

Rationale

In celo-blockchain transactions can be sent as arguments, and internal/ethapi/transaction_args.go creates the proper transaction type, and signed from an unlocked account. This is currently missing in op-geth.

Implementation

Implementation should be analog to celo-blockchain's implementation.

[ezdac]

Mainly putting this here for rubber-ducking, if you have opinions on this feel free to comment though

What is not completely clear to me is how to distinguish between Celo denominated (CIP66) and fee-currency denominated transactions (CIP64) in the transaction arguments, especially when the RPC method internally also sets sane defaults if fields are missing.
TransactionArgs do not have a type field where you could specify the exact transaction type, e.g. 0x02, so the type is inferred by the presence or lack of some fields.

Methods that will set sane defaults for some fields:

• eth_sendTransaction
• eth_createAccessList
• eth_signTransaction
• eth_fillTransaction

In general, without a qualifying field like gasPriceIsDenominatedInFeeCurrency, we have to make assumptions for the TransactionArgs:

• FeeCurrency == nil is conclusive for not using CIP-66 or CIP-64 transactions, normal EIP logic applies.
• FeeCurrency != nil && MaxFeeInFeeCurrency != nil is conclusive to assume a CIP-66 transaction, (however the client needs to know the gas-limit and gas price fields beforehand to fill that field)
• FeeCurrency != nil && MaxFeeInFeeCurrency == nil could mean several things:
  • if gas-price related fields and a gas limit are given, the client wants to send a CIP-64 transaction and the gas-price related values are denominated in FeeCurrency
  • if gas-price related fields are given, but no gas-limit is given, we default to a CIP-66 transaction and assume Celo denominated gas prices. We then estimate the gas-limit and give a suggestion for MaxFeeInFeeCurrency

The latter case has the downside, that legacy clients might assume fee-currency denominated gas-prices but simply haven't done an explicit gas-estimation. This could lead to overpaying gas-fees.
Just

How do we solve this ambiguity?

The easiest fix would be to make MaxFeeInFeeCurrency a required field for CIP-66 transactions.
We then would not offer MaxFeeInFeeCurrency suggestions - but then newer clients are forced to explicitly call
eth_estimateGas and eth_maxPriorityFeePerGas before being able to call aforementioned methods for Celo denominated transactions.
In general we want the API to behave intuitively especially for CIP-66 transaction, which this does not offer.

If we want to suggest a sane MaxFeeInFeeCurrency, how would we do that?
Consider the latest exchange rate and estimate the gas and then add e.g. +0.5% on the result?

TL;DR

In some JSON RPC methods the transaction type is inferred by lack or presence of some fields.
Some of those methods set sane defaults if some fields are missing, clients could rely on this.
For CIP-64 / CIP-66 transactions there is some ambiguity in some combinations of given / lacking fields. Especially it is not clear if a lack of MaxFeeInFeeCurrency means lack of knowledge about CIP-66 transactions or a hint to let the server determine a sane value.

Suggestion

It's probably best to not allow the CIP-66 estimations for now and make the clients do them with explicit calls (estimateGas, maxPriorityFeePerGas). Once CIP-64 transactions are deprecated, we can drop support for older clients and always assume Celo denominated gas prices as the basis for calculating sane defaults.

The problem with this approach is that the semantics of the API changes over time without proper versioning, and a clear cut upgrade might be hard to time with multiple client libraries.
We could later transition to a different namespace for Celo related transactions, or append a version string to the method name.

Another alternative would be to instead add a type field to the transaction-args, so that clients can clearly state their intentions on what transaction type behavior they assume. It seems that some clients like viem even send this with the transaction-args already.

[karlb]

@aaronmgdr came across the same problem for the client libraries. You might want to sync with him on this.

[aaronmgdr]

i didnt even consider that the geth client can receive tx as json as well as the rlp encoded.

this is interesting. I was thinking that maxFeeInFeeCurrency was required. but i suppose not for eth_fillTransaction.

FeeCurrency != nil && MaxFeeInFeeCurrency != nil is conclusive to assume a CIP-66 transaction, (however the client needs to know the gas-limit and gas price fields beforehand to fill that field)

client in this sentence meaning the one sending the tx? or the geth-client? and they need to know the gas and price because it is computed from them. yes makes sense

FeeCurrency != nil && MaxFeeInFeeCurrency == nil could mean several things:

1. if gas-price related fields and a gas limit are given, the client wants to send a CIP-64 transaction and the gas-price related values are denominated in FeeCurrency
2. if gas-price related fields are given, but no gas-limit is given, we default to a CIP-66 transaction and assume Celo denominated gas prices. We then estimate the gas-limit and give a suggestion for MaxFeeInFeeCurrency

for 1. I think they could be intending cip64 or cip66. and apart from guessing based on the values there really is no way to know.

[aaronmgdr]

if it helps none of the js libraries (contractkit, viem, ethers-wrapper) that support cip64 use the eth_fillTransaction rather txns missing fields are done filled calling eth_gasPrice(feeTokenAddres) and eth_maxPriorityFee

[aaronmgdr]

if gas-price related fields are given, but no gas-limit is given, we default to a CIP-66 transaction and assume Celo denominated gas prices. We then estimate the gas-limit and give a suggestion for MaxFeeInFeeCurrency

i dont understand the logic here. why would not having gas limit but having maxFeePerGas mean this was cip66 and not cip64?

[ezdac]

why would not having gas limit but having maxFeePerGas mean this was cip66 and not cip64?

It would be a compromise, to allow at least one combination of arguments to mean CIP64.

The reasoning here is that knowing all gas related fields you could already compute the MaxFeeInFeeCurrency but chose to not include it - hinting you don't know about CIP66 transactions. However it is questionable to what extent the procedures make sense at this point, because you could also do the encoding / signing client-side and use the eth_sendRawTransaction and wouldn't need the default filling behaviour.

---

I was thinking that maxFeeInFeeCurrency was required.

You are right, in the current celo-blockchain implementation a client (meaning the RPC caller) has to provide it for Celo denominated fee-currency transactions.

But should we require it for CIP-66 transactions? If we don't then we have to be comfortable with defining and using a default "slippage" for the eth_fillTransaction related procedures, determining how much the user can overpay due to exchange rate risk.

I think when you follow the normal Ethereum RPC logic one would expect the parameter to be optional - especially since making it required would also remove the gas-price and gas-limit calculation in those procedures, since they need to be known in advance anyways.
If we focus on the user experience going forward (CIP-66) I would vote for making it optional.

So you are saying all JS libraries that are Celo compatible use eth_sendRawTransaction and calculate gas related values beforehand? Are there other callers of the JSON RPC to consider, like indexers etc.?

If no, we don't need to consider backwards compatibility for those procedures and can prioritize CIP-66 usability.

[aaronmgdr]

So you are saying all JS libraries that are Celo compatible use eth_sendRawTransaction and calculate gas related values beforehand?

Viem

transactions missing fee price fields are filled using eth_gasPrice an eth_maxPriorityFeePerGas. typical eip 1559 txns get the data from block

a search for eth_fillTransactions in repo returns 0 results

celo-ethers-wrapper

transactions missing fee price fields are filled using eth_gasPrice an eth_maxPriorityFeePerGas

a search for eth_fillTransactions in repo returns 0 results

Contract Kit

all calls are eth_sendTransaction are intercepted and turned into eth_sendRawTransaction calls

transaction missing fee price fields are filled using eth_gasPrice an eth_maxPriorityFeePerGas

a search for eth_fillTransactions in repo returns 0 results

[aaronmgdr]

Are there other callers of the JSON RPC to consider, like indexers etc.?

MiniPay uses their own java based system for transaction serialization, sending rpc calling etc.

indexers would be read only so i dont see why they would care.

[lvpeschke]

@ezdac can you provide a status update? how much and what is left to do?

[ezdac]

@lvpeschke can you provide a status update? how much and what is left to do?

It will need around a half day of refining the PR #123 and then the review / potential requested changes revision.
I have to see when I will get to this because of the conference / offsite weeks.

[lvpeschke]

@ezdac can you provide another status update and an ETA?

[ezdac]

@lvpeschke no updates so far, since I prioritized other work. The PR will be finalized beginning of this week, the ETA then depends on review/requested changes.