Skip to content

Commit aa01376

Browse files
committed
added detailed description of edge cases encountered when used in foundry tests
1 parent ec9bbde commit aa01376

File tree

1 file changed

+29
-4
lines changed

1 file changed

+29
-4
lines changed

packages/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,36 @@ import {
2222
* This library mitigates that by providing a more convenient Solidity API for SuperTokens.
2323
* While most methods are just wrappers around equivalent methods in a Superfluid agreement,
2424
* some implement higher level abstractions.
25+
*
2526
* Note using the library in foundry tests can lead to counter-intuitive behaviour.
26-
* E.g. "prank" won't set the expected msg.sender for calls done using the library.
27-
* Also, reverts caused by the library itself won't be recognized by foundry, because
28-
* it expects them to happen in the context of an external call.
29-
* This is not specific to this library, but a general limitation of foundry when using libraries in tests.
27+
* 1) `prank` does not behave consistently with some methods.
28+
* Many library methods require the host address and/or agreement addresses in order to interact with this contracts.
29+
* This framework addresses are determined through the token contract (`token.getHost()`).
30+
* In order to avoid this gas cost overhead for future invocations, the library caches those addresses in storage.
31+
* A side effect of this optimization is that as long as this caching hasn't taken place
32+
* (which is the initial condition), `prank` will already be _consumed_ by the call fetching the host address,
33+
* not having any effect on the following call(s) which execute the actual action.
34+
* Possible mitigations:
35+
* - use `startPrank` instead of `prank`
36+
* - only use `prank` after doing a library call which did "warm up" the cache
37+
* 2) For some methods, `startPrank` doesn't change behaviour in the expected way either.
38+
* That affects all library methods using `address(this)`. The reason is as follows:
39+
* Some library methods are convenience wrappers which set the calling contract itself as sender.
40+
* Since the library code is executed in the context of the contract using it, _self_ means `address(this)`.
41+
* That however means that `prank` and `startPrank` won't override it.
42+
* Possible mitigations:
43+
* - if possible, design the test case such that the test contract itself can be the intended sender
44+
* - avoid using this convenience wrappers in tests, use methods with explicit sender argument instead
45+
* - create a helper contract which is the designated sender, and route calls through it
46+
* 3) `expectRevert` sometimes doesn't _see_ reverts.
47+
* `expectRevert` expects a revert in the next call.
48+
* If a revert is triggered by library code itself (vs by a call), `expectRevert` will thus not _see_ that.
49+
* Possible mitigations:
50+
* - avoid higher-level library methods which can themselves trigger reverts in tests where this is is an issue
51+
* - wrap the method invocation into an external helper method which you then invoke with `this.helperMethod()`,
52+
* which makes it an external call
53+
* Also be aware of other limitations, see
54+
* https://book.getfoundry.sh/cheatcodes/expect-revert
3055
*/
3156
library SuperTokenV1Library {
3257

0 commit comments

Comments
 (0)