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

FV (WIP do not merge) #26

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

FV (WIP do not merge) #26

wants to merge 19 commits into from

Conversation

d-xo
Copy link
Collaborator

@d-xo d-xo commented Jul 22, 2019

Some notes on current FV progress. Mostly here for handover reasons. The easy stuff is all done, but progress on plot / drop/ exec is very slow.

Code Changes

The following changes have been made to ease FV:

  • Switch from DSAuth to rely/deny (removes an external call)
  • Replace mapping (bytes32 => bool) with mapping(bytes32 => int) (avoid dealing with solidity bool masking)
  • Use bounded DSNote from dss (K cannot deal with unbounded msg.data size in the main DSNote)
  • Make plot/drop/exec external and inline the hash helper (solidity uses a for loop when copying bytes memory types from calldata, but can directly use CALLDATACOPY for bytes calldata types)

I am also considering removing the proxy as part of the work on exec. It prevents malicious plans from plotting new undelayed plans, but since plans can modify auth on the pause anyway, I'm not sure it actually very useful. It also feels a little too baroque now that I come back to look at it.

Dynamic Data

K has a really hard time with the dynamic data in the pause. There are two problematic areas that I have encountered so far:

  1. Computing the size of the call data. This requires a calculation that has symbolic inputs to chop and ceil32, both of which are concrete. I was able to figure out some (pretty ugly) custom lemmas that let K simplify these expressions.

  2. Computing memory expansion costs for CALLDATACOPY. This requires calculations with symbolic inputs to #memoryUsageUpdate, which is [concrete]. I was unable to find a set of lemmas that would help K simplify these expressions. These expressions are more complex than those in the size calculations and they simplify to something of the form X * VGas < A * size(fax)^2 + B * size(fax) + C) and so I cannot see an easy way to help K simplify these expressions when both VGas and fax are symbolic.

For this reason my current approach is to first verify the pause with a fixed size for fax and CD, which although obviously not exhaustive, still explores significantly more state space than the unit tests. This has so far not been successful either.

d-xo added 18 commits July 2, 2019 18:01
Solidity will use loops when abi encoding dynamic data from memory. This
makes FV much harder than needs to be. This commit ensures that only
CALLDATACOPY is used when working with dynamic data.

It also results in a modest gas saving.
Solidity inserts a bunch of weird masking bureaucracy when storing
bools. This makes FV unnecessarily complicated, so they are replaced
here with mappings to uints.
ds-auth is a pain for FV (and it's usage of an external call means that
we cannot fully verify it). Switching back to rely/deny for now.

Integration tests are left unupdated so as to focus on FV for now.
Some logging changes were required. dapphub/ds-note logs all calldata
which is incompatible with FV. For now we use the LogNote with bounded
calldata from dss on all functions that do not require logging of
unbounded calldata.

plot/drop/exec need to log unbounded calldata so custom events are used
on these functions.
There are some really tricky symbolic gas terms that arise from
calldatacopy and symbolic calldata.

This commit attempts to include the (symbolic) memory expansion gas cost
term into the gas term directly in the hopes that they will just cancel
out.

They didn't :(
@livnev
Copy link
Member

livnev commented Jul 22, 2019

For this reason my current approach is to first verify the pause with a fixed size for fax and CD, which although obviously not exhaustive, still explores significantly more state space than the unit tests. This has so far not been successful either.

I think this is a good way to start 👍

@d-xo
Copy link
Collaborator Author

d-xo commented Jul 28, 2019

So I fixed the calldata size to 64 in an if block. K still got stuck on the same calldatasize checking lemmas as before, and I spent quite a long time stuck trying to make it through.

If I readd the same (messy) lemmas that helped K out when the calldata size was bounded, I end up at some new mess of a gas term that I do not understand somewhere a little further along.

I've pretty much hit a brick wall here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants