What Are We Gonna Do When Blockchain State Changes During "Roles Edits" Proposals?

[adamgall]

unaffected user stories

• add hats
• role details change
• add stream

affected user stories

ASSUMPTION:

we (the app, at proposal creation time) assume that the state of the blockchain at proposal creation will stay unchanged (except for time passing) by the time proposal execution eventually happens.

the following scenarios will render one or more of the above set of assumptions invalid:

• role member changed
  • IF any streams end streaming OR any streams are cancelled DURING the proposal time
    • AND the original recipient CLAIMS their funds after the stream ends or is cancelled during this proposal time
      • THEN the proposal will attempt to withdraw funds from an empty Sablier stream
        • RESULTING IN A REVERTED TRANSACTION
  • IF any streams start streaming funds DURING the proposal time
    • THEN the proposal will not include a call to claim and withdraw from the Sablier stream at execution
      • RESULTING IN OUTGOING MEMBER FUNDS GOING TO THE INCOMING MEMBER
  • IF any streams are not actively streaming but DO have funds to withdraw from Sablier
    • AND the original recipient CLAIMS their funds from the stream DURING the proposal
      • THEN the proposal will attempt to withdraw funds from an empty Sablier stream
        • RESULTING IN A REVERTED TRANSACTION
• role deleted
  • IF any streams end streaming OR any streams are cancelled DURING the proposal time
    • THEN the proposal will attempt to cancel a stream which is already over or cancelled
      • RESULTING IN A REVERTED TRANSACTION
  • IF any streams start streaming funds DURING the proposal time
    • THEN the proposal will not include a transaction to withdraw any streamed funds from the newly cancelled Sablier stream
      • RESULTING IN LOST MEMBER FUNDS
  • IF any streams are not actively streaming but DO have funds to withdraw from Sablier
    • AND the recipient CLAIMS their funds from the stream DURING the proposal
      • THEN the proposal will attempt to withdraw funds from an empty Sablier stream
        • RESULTING IN A REVERTED TRANSACTION
• stream edited (not past end date and not cancelled)
  • IF stream ends OR stream is cancelled DURING the proposal time
    • THEN the proposal will attempt to cancel the stream which is already over or cancelled
      • RESULTING IN A REVERTED TRANSACTION
  • IF the stream start streaming funds DURING the proposal time
    • THEN the proposal will not include a transaction to withdraw any streamed funds from the newly cancelled Sablier stream
      • RESULTING IN LOST MEMBER FUNDS
  • IF the stream is not actively streaming but DOES have funds to withdraw from Sablier
    • AND the recipient CLAIMS their funds from the stream DURING the proposal
      • THEN the proposal will attempt to withdraw funds from an empty Sablier stream
        • RESULTING IN A REVERTED TRANSACTION
• stream cancelled
  • IF stream ends OR stream is cancelled DURING the proposal time
    • THEN the proposal will attempt to cancel the stream which is already over or cancelled
      • RESULTING IN A REVERTED TRANSACTION
  • IF the stream starts streaming funds DURING the proposal time
    • THEN the proposal will not include a transaction to withdraw any streamed funds from the newly cancelled Sablier stream
      • RESULTING IN LOST MEMBER FUNDS
  • IF the stream is not actively streaming but DOES have funds to withdraw from Sablier
    • AND the recipient CLAIMS their funds from the stream DURING the proposal
      • THEN the proposal will attempt to withdraw funds from an empty Sablier stream
        • RESULTING IN A REVERTED TRANSACTION

[adamgall]

Race Conditions Scenarios

• Role with at least one Payment that
  • has funds to be claimed
  • or not over
  • or not cancelled
• and the Role member is changing (including Role being deleted)

OR

• If a stream has already naturally ended, and we try to cancel it

The issues are

• that Sablier will revert if a Stream is attempted to be withdrawn from, and there are no funds to claim in that stream. Calling withdrawMax on a stream with no funds to claim will revert
• attempting to cancel a stream which has naturally ended will revert

Ok it seems like the thing to start hacking on here is:

• Anywhere in the Roles proposal builder where we’re calling withdrawMax and cancelStream move that (and any necessary context) into a new Module contract and dynamically make that decision to withdraw or cancel or not at runtime.

[DarksightKellar]

Test 1 - Active stream at time of proposal to cancel (cancel should revert on execution)

• Create a stream that ends in 10 mins, vote and execute quickly
• Create a proposal to cancel the stream, before the stream ends
• Vote yes, but wait for the stream to end.
• The proposal, as yet unexecuted, still thinks the stream has not ended
• Execute the proposal. This should revert as it tries to cancel and/or flush an already ended stream
  Result: I GET A TRANSACTION LIKELY TO FAIL ERROR FROM METAMASK
  [Fixed in Streams PRs. Proposal: proposals/28?dao=sep:0x5b68d217Be7e7FDfC80812D5D518767b19f2D933]

Test 2 - Inactive stream with funds at the time of proposal to change member (withdrawMax should revert on execution)

• Go edit a role that has a stream with unclaimed funds on it. The stream itself should be over.
• Update just the member.
• Vote yes on the resulting proposal, but don't execute yet.
• Have the original recipient of the stream withdraw their funds.
• The proposal, as yet unexecuted, still thinks the stream has unclaimed funds on it.
• Execute the proposal. This should revert as it tries to withdraw the unclaimed-but-actually-claimed funds to the outgoing recipient
  Control: Before trying to repro, clicking execute yielded estimation fee of 0.12935471 ETH
  Result: I GET A TRANSACTION LIKELY TO FAIL ERROR FROM METAMASK
  Control 2: Create another proposal to edit the role's member. This time the proposal is aware that the stream has no funds on it. It should pass.
  [Fixed in Streams PRs. Proposal: proposals/25?dao=sep:0x5b68d217Be7e7FDfC80812D5D518767b19f2D933]

Test 3 - Active stream at the time of proposal to change member (cancel should revert on execution if stream is cancelled just prior)

• Go edit a role that has an active stream on it.
• Update just the member.
• Vote yes on the resulting proposal, but don't execute yet.
• Edit the same proposal, but cancel the stream this time.
• Vote yes on the cancel proposal and execute ASAP.
• The edit-member proposal, as yet unexecuted, still thinks the stream is active.
• Execute the proposal. This should revert as it tries to cancel the active-but-actually-cancelled stream
  Result: I GET A TRANSACTION LIKELY TO FAIL ERROR FROM METAMASK
  Control: Do the same as above, but execute the edit-member proposal before the cancel proposal. The cancel proposal is what should instead revert.

---

• After cancelling a stream successfully, the member is updated to the safe. When trying to change the member from there on, there's always a revert. [should be fixed (because member is no longer updated after cancel)... but got blocked from specifically testing by weird metamask error/bug]