feat: ethrpc: implement call, estimateGas, getTransactionCount #9306
Conversation
|
I’m a little concerned that the gas estimation result is slightly different from
It seems to me that the difference is the execution context (e.g. the state of stmgr). @raulk any idea about this? |
node/impl/full/eth.go
Outdated
| receipt := api.NewEthTxReceipt(tx) | ||
| err = receipt.PopulateReceipt(msgLookup, replay) | ||
| if err != nil { | ||
| return api.EthTxReceipt{}, err | ||
| } |
There was a problem hiding this comment.
Nit: splitting these into two methods doesn't really seem necessary.
api/eth_types.go
Outdated
| func CheckContractCreation(lookup *MsgLookup) (*EthAddress, error) { | ||
| var result init8.ExecReturn | ||
| ret := bytes.NewReader(lookup.Receipt.Return) | ||
| if err := result.UnmarshalCBOR(ret); lookup.Receipt.ExitCode.IsSuccess() && err == nil { |
There was a problem hiding this comment.
Shouldn't we check IsSuccess first (no need to decode?)?
| BlockNumber EthInt `json:"blockNumber"` | ||
| TransactionIndex EthInt `json:"transacionIndex"` | ||
| From EthAddress `json:"from"` | ||
| To *EthAddress `json:"to"` |
There was a problem hiding this comment.
Why is this a pointer? I believe that from is optional because the node would use its default account in that case, but didn't know that to can be optional?
There was a problem hiding this comment.
@raulk to should be optional, because if the tx creates a contract, ethereum's behavior is to return "to": null for both the tx and the tx receipt
There was a problem hiding this comment.
Ah, true, I had forgotten about that edge case.
| var result init8.ExecReturn | ||
| ret := bytes.NewReader(lookup.Receipt.Return) | ||
| if err := result.UnmarshalCBOR(ret); err == nil { | ||
| contractAddr, err := EthAddressFromFilecoinIDAddress(result.IDAddress) |
There was a problem hiding this comment.
nit (can save for a future PR): should probably return a specific error message instead of letting it fall through to the "not a contract creation tx" error.
Related Issues
ref-fvm #854
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, testarea: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps