eth-brownie / brownie

A Python-based development and testing framework for smart contracts targeting the Ethereum Virtual Machine.
https://eth-brownie.readthedocs.io
MIT License
2.65k stars 552 forks source link

Improvements to state machines #509

Open iamdefinitelyahuman opened 4 years ago

iamdefinitelyahuman commented 4 years ago

Overview

Improve the design of Brownie state machines.

This is a work in progress, if you have any ideas feel free to leave them as comments or reach out on Gitter.

Specification

Example

This is a condensed version of the example state machine given in the documentation:

class OldStateMachine:

    value = strategy('uint256', max_value="1 ether")
    address = strategy('address')

    def __init__(cls, accounts, Depositer):
        # deploy the contract at the start of the test
        cls.accounts = accounts
        cls.contract = Depositer.deploy({'from': accounts[0]})

    def setup(self):
        # zero the deposit amounts at the start of each test run
        self.deposits = {i: 0 for i in self.accounts}

    def rule_deposit(self, address, value):
        # make a deposit and adjust the local record
        self.contract.deposit_for(address, {'from': self.accounts[0], 'value': value})
        self.deposits[address] += value

    def invariant(self):
        # compare the contract deposit amounts with the local record
        for address, amount in self.deposits.items():
            assert self.contract.deposited(address) == amount

def test_stateful(Depositer, accounts, state_machine):
    state_machine(OldStateMachine, accounts, Depositer)

Here is the same state machine, rewritten with the proposed new functionality:

@state_machine(Depositor, accounts)
class NewStateMachine:

    value = strategy('uint256', max_value="1 ether")
    address = strategy('address')

    def setup_first(cls, accounts, Depositer):
        # deploy the contract at the start of the test
        cls.contract = Depositer.deploy({'from': accounts[0]})

    def setup(self, accounts):
        # zero the deposit amounts at the start of each test run
        self.deposits = {i: 0 for i in accounts}

    def rule_deposit(self, accounts, address, value):
        # make a deposit and adjust the local record
        self.contract.deposit_for(address, {'from': accounts[0], 'value': value})
        self.deposits[address] += value

    def invariant(self):
        # compare the contract deposit amounts with the local record
        for address, amount in self.deposits.items():
            assert self.contract.deposited(address) == amount

Dependencies

Breaking change. Can happen as a part of v2.0.0. Also, this means plenty of time to bikeshed about the design :smile:

fubuloubu commented 4 years ago

setup_first is a bad name for explaining what it does. How about create_snapshot?

fubuloubu commented 4 years ago

Also, injecting the fixtures is pretty magical. Is it all the fixtures that are available, or only the ones I pull in via my test case where I initialized it?

iamdefinitelyahuman commented 4 years ago

It's magical, but it's the same magic that we're used to dealing with via pytest. In my mind it moves the structure closer to that of writing a normal test.

It would have to be limited to the fixtures that were requested in the test case... I don't think I'm able to grab a fixture outside of the local namespace of the test.

fubuloubu commented 4 years ago

I don't think I'm able to grab a fixture outside of the local namespace of the test.

How do you think pytest knows about fixtures in the first place?

iamdefinitelyahuman commented 4 years ago

Per gitter convo: maybe do away with the need to have a test_stateful at all, and instead use a class decorator to indicate a state machine. The fixtures can be given as input args to the decorator:

@state_machine(Depositor, accounts)
class NewStateMachine:
iamdefinitelyahuman commented 4 years ago

setup_first is a bad name for explaining what it does. How about create_snapshot?

I thought setup_first to mirror the teardown and teardown_final methods (which maybe should be included in the example).

Perhaps...

fubuloubu commented 4 years ago

What about:

Pros:

  1. Creates a delineation between snapshots (happens once) vs. cases (happens N times)
  2. Doesn't confuse the purposes of the two functions (one deploys smart contracts, the other deals with internal state)
iamdefinitelyahuman commented 4 years ago

Related - it should be possible to selectively disable coverage analysis during a stateful test. Either on a per rule/invariant basis, or a broad "disable during invariants".

iamdefinitelyahuman commented 4 years ago

Also related - add support for @precondition: https://hypothesis.readthedocs.io/en/latest/stateful.html#preconditions

Not sure where this fits in with the given syntax, it might still make the most sense as a decorator.

iamdefinitelyahuman commented 4 years ago

Would also be useful to allow a rule to have a "weight", i.e. making a specific rule twice as likely to run as the others.

Or a sort-of hybrid invariant/rule that executes after every rule but prior to the invariants

fubuloubu commented 4 years ago

I was just looking at this: https://hypothesis.readthedocs.io/en/latest/stateful.html#hypothesis.stateful.consumes

iamdefinitelyahuman commented 4 years ago

~It should be possible to use strategies within the setup_case method.~

It should be possible to create a type of initialize rule that accepts strategies and always executes, always prior to any rules. This should be seperate from setup_case - an unhandled exception during setup_case indicates a failure in the state machine itself.

fubuloubu commented 4 years ago

The ability to feature strategies based on state of a StateMachine:

class StateMachine:
    st_account = strategy("accounts")
    st_amount = strategy("uint256", stateful_exclude=lambda val, self: val < self.token.totalSupply())

...
    def rule_mint(self, a="st_account", val="st_amount"):
         self.token.mint(a, val)

    def rule_transfer(self, a1="st_account", a2="st_account", val="st_amount"):
         self.token.transfer(a2, val, {"from": a1})
fubuloubu commented 4 years ago

Allow printing something only when an invariant is violated (maybe some StateMachine.display function that executes when a falsifying example is found for each step along the way)

nazariyv commented 3 years ago

Stumbled on this issue by reading hypothesis docs and finding precondition and thinking it would be nifty to have that in brownie. Right now, I am hardcoding the conditions when the rule is allowed to fire in the body of the rule of the function, which must be wrong. I imagine @precondition in hypothesis is used to better guide the rule firing.