ApeWorX / ape-vyper

Vyper compiler plugin for the Ape Framework, using VVM
https://www.apeworx.io/
Apache License 2.0
25 stars 9 forks source link

feat: add dev message for nonpayable and safe math checks [APE-784] #73

Closed antazoey closed 1 year ago

antazoey commented 1 year ago

What I did

adds custom dev message for payable sent to non-payable so it can be asserted on using ape.reverts.

How I did it

copied brownie, sorta

How to verify it

Checklist

antazoey commented 1 year ago

feeling better now that safe math is working

fubuloubu commented 1 year ago

Otherwise, it's looking good to me

antazoey commented 1 year ago

Not to completely throw a monkey wrench in it, but I actually wonder if it might be better to raise a more descriptive error up the stack instead of raising as a dev assert message, like a built-in compiler plugin exception that it detects via pattern matching according to known compiler semantics

Ya definitely would be ideal, just not sure exactly how to make that work. Also, would maybe break how ape.reverts works

fubuloubu commented 1 year ago

Also, would maybe break how ape.reverts works

Even if they subclassed ContractLogicError?

fubuloubu commented 1 year ago

Just sketching an idea here, but it could look like taking the ContractLogicError exception, and then asking the compiler plugins if they can enrich that exception with a better error message that subclasses ContractLogicError


Edit, this might also make sense for how "custom errors" work with Solidity: https://github.com/ApeWorX/ethpm-types/blob/main/ethpm_types/abi.py#L352

antazoey commented 1 year ago

@fubuloubu Ya, that may be pretty cool. Let's see what we can do...

I think I still will need the pc -> error map thing tho for the source trace back / coverage stuff.

antazoey commented 1 year ago

Love the enrich_error idea and have that in the works. Still need these error PCs somewhere tho, so I think for now, we should let both plans work.

Have others PRs now for enrich error but need this one to merge first.

antazoey commented 1 year ago

Just sketching an idea here, but it could look like taking the ContractLogicError exception, and then asking the compiler plugins if they can enrich that exception with a better error message that subclasses ContractLogicError

Ok think I have it set up nicely here: https://github.com/ApeWorX/ape-vyper/pull/74 will be able to merge after next release. In the mean time, I added a comment , dev prefix, and renamed the enum to try and find the best middle ground.