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

Can add detect function support payable ? #6

Closed
CaiJiJi opened this issue Jul 31, 2024 · 14 comments
Closed

Can add detect function support payable ? #6

CaiJiJi opened this issue Jul 31, 2024 · 14 comments
Labels
enhancement New feature or request

Comments

@CaiJiJi
Copy link

CaiJiJi commented Jul 31, 2024

thanks

@cdump
Copy link
Owner

cdump commented Jul 31, 2024

It's trivial (at least for Solidity). I'll add this to the backlog and implement it later.

@cdump cdump added the enhancement New feature or request label Jul 31, 2024
@wtdcode
Copy link
Contributor

wtdcode commented Aug 24, 2024

It's trivial (at least for Solidity). I'll add this to the backlog and implement it later.

Maybe we can consider prioritizing this? This helps generate a somehow "full" abi.

@cdump
Copy link
Owner

cdump commented Aug 24, 2024

prioritizing this

After #9, it will be easier to implement new features only in 1 language.
I'm on 4'th step of TODO list

@wtdcode
Copy link
Contributor

wtdcode commented Aug 24, 2024

I can help implement this so that evmole can return a full Function

@cdump
Copy link
Owner

cdump commented Aug 24, 2024

Do you want to return https://docs.rs/alloy-json-abi/0.8.0/alloy_json_abi/struct.Function.html ? If yes, what about outputs field, which is not Optional, we don't have this information. Using empty Vec there may mislead the users.

@wtdcode
Copy link
Contributor

wtdcode commented Aug 24, 2024

Oh, you are correct. My own code always ignores this field so I forget this. Maybe another standalone function to return if a selector is payable or not.

@cdump
Copy link
Owner

cdump commented Aug 27, 2024

I've conducted some experiments in the state_mutability branch, the main idea is described here (typo: it's call_value=0 in 2.1.)

Using the largest1k dataset, I couldn't achieve more than a 53% success rate (counting view & pure from etherscan as payable, and only 33% if nonpayable (it's right)), which is far from satisfactory.

We need to explore alternative approaches.

P.S. In modern Vyper, we can extract this information from metadata, but let's focus on solving the Solidity issue first.

@cdump
Copy link
Owner

cdump commented Aug 28, 2024

I've added normal benchmark to 'state_mutability' branch, current results:

'simple' always returns 'nonpayable'

strict cmp ('view' != 'nonpayable'):

dataset largest1k (1000 contracts, 24427 functions), evmole-rs:
  bad:  20846 functions 85.34%

dataset largest1k (1000 contracts, 24427 functions), heimdall-rs:
  bad:  13403 functions 54.87%

dataset largest1k (1000 contracts, 24427 functions), sevm:
  bad:  14417 functions 59.02%

dataset largest1k (1000 contracts, 24427 functions), simple:
  bad:  14864 functions 60.85%

non-strict cmp ('view' and 'pure' == 'nonpayable') - source

dataset largest1k (1000 contracts, 24427 functions), evmole-rs:
  bad:  16177 functions 66.23%

dataset largest1k (1000 contracts, 24427 functions), heimdall-rs:
  bad:  6201 functions 25.39%

dataset largest1k (1000 contracts, 24427 functions), sevm:
  bad:  501 functions 2.05%

dataset largest1k (1000 contracts, 24427 functions), simple:
  bad:  643 functions 2.63%
$ cd benchmarks/
$ DATASETS=largest1k PROVIDERS_MUTABILITY="simple evmole-rs sevm heimdall-rs" make benchmark-mutability
$ python3 ./compare.py --datasets largest1k --mode mutability

@wtdcode
Copy link
Contributor

wtdcode commented Aug 28, 2024 via email

@cdump
Copy link
Owner

cdump commented Aug 28, 2024

I found a bug - we need to track reverts even inside 'skip_until_inside_function', because if all functions are not payable, then Solidity checks CALLVALUE at the beginning of the contract execution.
After the fix, we got:
bad: 59 functions 0.24% in non-strict mode and bad: 14,278 functions 58.45% in strict mode.
Now we need to improve strict mode.

@cdump
Copy link
Owner

cdump commented Aug 28, 2024

Bug was about location of this check - at the begging of the whole contract or after checking for function selector - inside function body

@wtdcode
Copy link
Contributor

wtdcode commented Aug 28, 2024 via email

@cdump
Copy link
Owner

cdump commented Aug 30, 2024

@cdump
Copy link
Owner

cdump commented Aug 31, 2024

Merged to the master, will make release soon

@cdump cdump closed this as completed Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants