Separate current state from current action in Branching Trees

[PaulRBerg]

Problem

Copy-pasting @DrakeEvans's feedback from a private chat on Telegram:

I mostly see the "when" keyword on the branching tree here, do you differentiate between a given state and action in the .tree files? I typically think of tests in the Arrange, Act, Assert phases which correlate to Given, When, Then in Cucumber style notation. But it looks like you are combining the current state of the system and the current action into a single statement like "when the admin is not the same as the current admin" (line 8). I think this is a little confusing (or at least it was to me on the first read) because it's not clear in which instances we are talking about function arguments and where we are talking about the state of the system without context clues. And admittedly this situation wasn't too hard to parse.

Looking at transferAdmin:

• On line 2, "the admin" refers to the currently set admin (state of the system)
• On line 8, "the admin" refers to the function parameter, and "current admin" is referring to the state of the system

To be clear I really like the branching idea for keeping the information DRY, but I wonder if there is a way to make it slightly more explicit without the need for a full description of the prior state which can be verbose. Some way to delineate between how the action takes place and the current state prior to an action

Drake is absolutely right - our tree terminology commingles the current state (contract state) with the current action (function arguments). It would be helpful to draw a distinction between the two

Potential Solution

Follow Cucumber's Gherkin syntax and replace "when" with "given" for the tree nodes that refer to the contract state.

Related

• ETHDenver talk: Better Smart Contracts with Behavior-Driven Development

[ShubhSensei]

Can I work on this issue?

[PaulRBerg]

Hey @SensationalShubham! This is a good first issue to work on.

Could we do it this way?

1. You start the refactor on the unit tests (and only them)
2. You open a draft PR
3. We review it
4. We continue refactoring the rest of the code base

[PaulRBerg]

@andreivladbrg - could you pick up this task? You can start with the integration tests for the Comptroller, and make the test/improve-tree-wording branch the base of your PR.

Skimming through the Gherkin Reference might help with this task.

[PaulRBerg]

#641 was great, thanks @smol-ninja.

If you would like to make further contributions, you could continue with this refactor here:

• https://github.com/sablier-labs/v2-core/tree/fcbc567aea6362c1f102eb19732badeb4899dec5/test/integration/concrete/nft-descriptor
• https://github.com/sablier-labs/v2-core/tree/main/test/integration/concrete/flash-loan

As well as the fuzz tests for the same contracts.

[alexfertel]

@PaulRBerg @andreivladbrg Just to confirm, the change to the syntax of trees would be that some whens are now givens and that reflects in the contracts as a substitution as well?

[PaulRBerg]

Precisely right, @alexfertel.

You can see some related commentary in #647.