Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lotus FEVM API Incompatibility with normal Ethereum mainnet behavior, causing issues with integration partners #12441

Closed
4 of 7 tasks
Schwartz10 opened this issue Sep 10, 2024 · 6 comments · Fixed by filecoin-project/fevm-contract-tests#28
Assignees
Labels

Comments

@Schwartz10
Copy link
Contributor

Checklist

  • This is not a security-related bug/issue. If it is, please follow please follow the security policy.
  • I have searched on the issue tracker and the lotus forum, and there is no existing related issue or discussion.
  • I did not make any code changes to lotus.

Lotus component

  • lotus Ethereum RPC
  • lotus FVM - Lotus FVM interactions
  • FEVM tooling
  • Other

Lotus Version

Running the public glif nodes

Repro Steps

Curl request to eth_estimateGas that fails with actor not found:

curl 'https://api.calibration.node.glif.io/rpc/v1' \
  -H 'Accept: application/json' \
  -H 'Accept-Language: en-US,en;q=0.9,es;q=0.8' \
  -H 'Connection: keep-alive' \
  -H 'Content-Type: application/json' \
  -H 'Cookie: __stripe_mid=778e36cb-7efb-46da-986b-8e8e1146f17e7602bc' \
  -H 'Origin: chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/' \
  -H 'Sec-Fetch-Dest: empty' \
  -H 'Sec-Fetch-Mode: cors' \
  -H 'Sec-Fetch-Site: none' \
  -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36' \
  --data-raw '{"id":588633187,"jsonrpc":"2.0","method":"eth_estimateGas","params":[{"from":"0xb1040d3ce131b3204e5229c80a8d5ae271b2ef09","to":"0x7fa33ed9fce1e9c620cef778c5601286cd68e027","data":"0x5c19a95c000000000000000000000000b1040d3ce131b3204e5229c80a8d5ae271b2ef09"}]}'

returns:

{"jsonrpc":"2.0","error":{"code":1,"message":"failed to estimate gas: ApplyWithGasOnState failed: call raw get actor: resolution lookup failed (t410fweca2phbggzsatssfheavdk24jy3f3yjxzll22i): resolve address t410fweca2phbggzsatssfheavdk24jy3f3yjxzll22i: actor not found"},"id":588633187}

This is an incompatibility with the same call on infura ethereum mainnet, which returns:

{"jsonrpc":"2.0","id":3525842942,"result":"0xbea9"}

Describe the Bug

Integration partners call eth_estimateGas to estimate the gas of a given transaction. In some circumstances, a web app may estimate the gas of a transaction even if the account has no balance to pay for the transaction fee. Ethereum mainnet seems to ignore the from address' balance when calling eth_estimateGas . Subsequently, when the same user tries to submit the transaction with the same from address, the call fails with insufficient funds (or maybe in our case, actor not found).

In some cases, integrators have to patch filecoin specific functionality into their otherwise chain agnostic frontend

Tooling

Curl

Configuration Options

This is the public glif api, i'm not sure its exact config options
@Schwartz10 Schwartz10 added the kind/bug Kind: Bug label Sep 10, 2024
@rvagg
Copy link
Member

rvagg commented Sep 10, 2024

I've been trying to revive fevm-contract-tests for more regular runs against the latest lotus master and I've been running into this problem there too. I have 8 failures on the uniswap test suite, and all but 1 are variations of this bug.

  1) UniswapV3Factory
       #setOwner
         fails if caller is not owner:
     AssertionError: Expected transaction to be reverted, but other exception was thrown: ProviderError: failed to estimate gas: ApplyWithGasOnState failed: call raw get actor: resolution lookup failed (f410foji4f3nrgqcrctymqwe5gmppms2nvy6iqkoyaty): resolve address f410foji4f3nrgqcrctymqwe5gmppms2nvy6iqkoyaty: actor not found

So I guess we've introduced some more strict checking at some point in the pipeline here.

@jennijuju
Copy link
Member

should try to repro w/ metamask and see what error response other evm chain returns (see the warning)
image

@Schwartz10
Copy link
Contributor Author

image Ethereum mainnet

@rvagg
Copy link
Member

rvagg commented Oct 4, 2024

hah, this call is working now on calibnet

{"jsonrpc":"2.0","result":"0x925da8","id":588633187}

the actor has funds, or has an actorid now, or ... something?

if we modify the from manually to be some unknown address and we get the same error, e.g. changing the 9 on the end to an 8, getting:

{"jsonrpc":"2.0","error":{"code":1,"message":"failed to estimate gas: ApplyWithGasOnState failed: call raw get actor: resolution lookup failed (t410fweca2phbggzsatssfheavdk24jy3f3yixfykv3a): resolve address t410fweca2phbggzsatssfheavdk24jy3f3yixfykv3a: actor not found"},"id":588633187}

does geth / infura / whoever, still process it without the valid from? I don't have an eth endpoint to test this.

I'm assuming that we inserted an actor check in there at some point that shouldn't be there, but I'm honestly not sure what the 'correct' behaviour is. @Schwartz10 ?

@Abhay-2811
Copy link

Hey @rvagg so I tried the calls and only the calls [Example: eth_call and eth_estimateGas ] that use from as a param throw this exact error. the error boils down to the issue with following piece of code:

[// LookupIDAddress gets the ID address of this actor's addr stored in the InitActor.](

// LookupIDAddress gets the ID address of this actor's `addr` stored in the `InitActor`.
func (st *StateTree) LookupIDAddress(addr address.Address) (address.Address, error) {
if addr.Protocol() == address.ID {
return addr, nil
}
resa, ok := st.snaps.resolveAddress(addr)
if ok {
return resa, nil
}
a, err := st.lookupIDFun(addr)
if err != nil {
return a, err
}
st.snaps.cacheResolveAddress(addr, a)
return a, nil
}
)

A quick patch would be: checking for calls that don't require gas [ I only came across 2 so it's feasible ] and remove the from param from call.

Do let me know your thoughts about this

@rvagg
Copy link
Member

rvagg commented Oct 18, 2024

@akaladarshi has been looking into this one too, latest thread on Slack.

@akaladarshi would you mind dropping in a summary of your findings so far? I thought your findings from go-ethereum were the most interesting bit—it basically shortcuts some of the internal checks and if we were to follow a similar path it would mean sending that into the fvm to have it avoid some of its checks. But I think we need to work out what all of those checks are first.

We really should build a solid, failing itest for this first that we can work on fixing. There are possibly more than one situation that we need to account for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🎉 Done
Development

Successfully merging a pull request may close this issue.

4 participants