Zondax / filecoin-solidity

Filecoin Solidity API Library
Apache License 2.0
93 stars 43 forks source link

should `MinerAPI.getXXXX` be `view`? #359

Closed hcheng826 closed 1 year ago

hcheng826 commented 1 year ago

Or this function is expected to change state?

Including getOwner, getSectorSize, getAvailableBalance, getVestingFunds and so on.

:link: zboto Link

maciejwitowski commented 1 year ago

These functions aren't expected to modify the state. They all past the static_call = true flag. Eg here (the last argument).

hcheng826 commented 1 year ago

Thanks for the reply! Then shouldn't it be marked as view? Also I tried to deploy this contract to testnet and call the function. Looks like it cost gas to do so. Is this expected? image

The transaction: https://hyperspace.filfox.info/en/message/bafy2bzaced4775cvskrqjgr6tlfjjwmmb37l7gbniotyf3h7iem5viasncbzy

maciejwitowski commented 1 year ago

Yeah, this is tricky. You can see the explanation in this Slack thread, especially

solc's mutability checker sees a "delegatecall" and decides the method can't be view, because a delegatecall might be able to change state.

and

If you want to mark those methods view, you have to fool the mutability checker.

Here is the example of "fooling the mutability checker".

rllola commented 1 year ago

I would not recommend doing this hack in your code to fool the checker. But indeed we can't put view on functions because all the call use DelegateCall opcode.

And to answer your second question, simply getting a value cost gas. In the PR you can see a gas consumption table (https://github.com/Zondax/filecoin-solidity/pull/358#issuecomment-1473998338). The values are taken from running the contract in a local fvm.