openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

base class for the "till" pattern

Open moodysalem opened this issue 1 year ago • 4 comments

🧐 Motivation The till pattern that utilizes transient storage is useful for many DeFi protocols.

📝 Details

Create a base contract that implements the till pattern, that can be inherited by other contracts. In this pattern:

  • A caller calls the contract and then receives a callback with a new ID
  • Within the callback, the caller can make any number of changes (need a modifier and helper function to get current caller ID and prevent calling other methods without a lock)
  • At the end of the till, run some abstract validation to ensure the initial call can return successfully
  • Note in this pattern, re-entrance is allowed and the nested caller gets a new ID
  • Caller must be able to specify input to the callback and receive output from the callback

moodysalem avatar Jun 16 '23 15:06 moodysalem

I'm not sure that I understand the till metaphor. Is there an implementation in Uniswap v4 that could be used as reference?

Can the same caller reenter via the till? Is the validation executed on entry as well? (If not, that seems risky?)

frangio avatar Jun 16 '23 16:06 frangio

I'm not sure that I understand the till metaphor. Is there an implementation in Uniswap v4 that could be used as reference?

It's a till, like a cash register, in that you open it, take some cash out, put some cash in, maybe change currencies (examples of actions you can perform while the till is open), but it has to be balanced (an example of the abstract validation) when you close it (the call returns). Another way to think of it is you don't perform validation of invariants until the end.

Can the same caller reenter via the till? Is the validation executed on entry as well? (If not, that seems risky?)

Yes, any caller including the current caller can re-enter any number of times. Each call is assigned a unique ID--unique only within the context of that call stack--for each call to open the till. The abstract validation is only executed just before return. As long as the transient state you validate at the end is related only to that caller's ID, it's plenty safe.

moodysalem avatar Jun 16 '23 18:06 moodysalem

As long as the transient state you validate at the end is related only to that caller's ID, it's plenty safe.

Can you elaborate on this?

This pattern seems to break some of my assumptions about what secure reentrancy looks like. It sounds like an invariant can be broken in the middle of the callback execution as long as it's restored at the end. But if you allow reentrancy while invariants are broken, the preconditions that you would normally use to prove that the code does what you intend may not hold. I'm not saying that the pattern can't work, but it seems to have big implications.

frangio avatar Jun 16 '23 22:06 frangio

As long as the transient state you validate at the end is related only to that caller's ID, it's plenty safe.

Can you elaborate on this?

This pattern seems to break some of my assumptions about what secure reentrancy looks like. It sounds like an invariant can be broken in the middle of the callback execution as long as it's restored at the end. But if you allow reentrancy while invariants are broken, the preconditions that you would normally use to prove that the code does what you intend may not hold. I'm not saying that the pattern can't work, but it seems to have big implications.

Your invariants that you typically check are based on the state at the beginning of that specific call. The easiest way to do this is to only track the differences. This would be the responsibility of the implementor, as part of the abstract validation.

moodysalem avatar Jun 17 '23 00:06 moodysalem