Questions & Suggestions For Super Pool V1

[saflamini]

Overview

First off, here’s my general understanding of how this works. Please correct me if I am wrong about this @donoso-eth

You can send funds to a given super pool contract by doing the following 1) Acquire super tokens by wrapping an existing ERC20 (if you already have super tokens in your wallet, great) 2) Calling send() on the super token you’re working with to send these tokens into the pool you’re interacting with or creating a stream of tokens to the pool you’re interacting with Correct 👍

Once you’ve sent super tokens into a pool contract, you’ll receive spTokens in return Correct 👍

• For the entire time that a user has their tokens sitting inside of a super pool strategy, the user will see their balance of spTokens increase. Correct 👍

• These tokens are delivering yield to users according to a pre-defined strategy. The Super Pool is designed such that the strategies are configurable. For example, devs using this template could create strategy contracts which plug in to existing defi protocol that enables users to deposit tokens & earn yield Correct 👍

Users can redeem their underlying tokens using either a stream or lump sum withdrawal. However, there are some limits on which of these you can use depending on the circumstance:

• A user may lump sum deposit in and lump sum withdraw (aka - withdraw their underlying super tokens and burn spTokens) Correct if the underlying refers to the supertoken as underlying from spTokens!!

• A user can stream in and lump sum redeem (aka - withdraw their underlying super tokens and burn spTokens) Correct if the underlying refers to the supertoken as underlying from spTokens!!

• A user can lump sum in and stream redeem (aka - withdraw their underlying super tokens and burn spTokens) Correct if the underlying refers to the supertoken as underlying from spTokens!!

• A user cannot stream in and stream redeem (aka withdraw in a stream) Correct 👍

Questions

General

• What is the roadmap here? Are there any new functions/features that need to be finished before we are able to say that this is ready to be used on testnet?

  • if so, what are they? 👉 Emergency pause 👉 Close Account already implemented

  • Note: our goal is to turn this project into the ultimate Template for Sophisticated RealTime Finance applications

• I see several commented out pieces of code throughout. can we delete these to avoid confusion? 👍 cleaned

• Can we use the new SuperTokenV1Library instead of CFALibrary? Will save a lot of boilerplate and be much more readable. I can help with this and you can find the library here 👍 will check

• Is PoolV1 meant to inherit from PoolInternal? Or is PoolInternal its own separately deployed contract? 👉 yes both are separated contracts pool-internal works as a library, there are separated due to bytes size. We use delegateCall() from PooV1 to communicate with PoolInternal to preserve the calling context

• inside of PoolV1, we inherit from PoolState, but not PoolInternal. PoolInternal then inherits from PoolState? 👉 exactly, PoolState is abstract contract defining the contract state, in order to ensure that the delegatecalls do not break, we inherit the PoolState so PoolV1 and PoolInternal have the same storage layout

• Can we get better labeling on the test names vs user-expected 1-n? Because we’re using this json format, it’s really hard to figure out what each test is actually checking 👉 Sorry for that, you are completely right, in order to reproduce the same scenarios I tested on hadhat I exported from hardhat the results in json and used with foundry, without further info, it is impossible to understand!!

Pool-V1 Contract

• Can we document the usage of assembly in callInternal() ? I.e. what is actually happening here? 👉 yes, I will do. As delegateCall() do not revert, only returns success = false, this is a way to get revert message if any in this case

• What happens if I pass 0 to _redeemFlow? Does it just do what _redeemFlowStop does? 👉 redeemFlow(0) should revert

  • it looks like it might, but balanceTreasury() is not called in that case?

• There are two instances of transfer(), one of which seems to be defined with an unlabeled parameter address , and another which calls gelato in its implementation 👉 corrected, changed the payment to gelato method

  • there’s also an implementation oftransfer() commented out, and one version of transfer has 2 lines commented out in its implementation (lines 575/576) 👉 cleaned

• Is something supposed to be happening in beforeTokenTransfer and afterTokenTransfer? I don’t see an implementation. And it also seems like the necessary functionality is already happening in PoolInternal’s transferSTokens? 👉 cleaned

• When I want to transfer my spTokens, what function will I call? 👉 normal erc20 transfer

• Should transferSTokens be an external function? Or should it be internal if it’s only meant to be called later down in the stack by transfer() on the pool contract? Or am I misunderstanding what is happening here? 👉 You are right, this method overrides the transfer erc20 functionality updating the pool state and can only be called when the erc20 transfer is called

• TransferSTokens - shouldn’t this be ‘transferSPTokens’ if we’re calling the pool tokens spTokens? 👉 Much better, changed

• Lines 116-125 in Pool - what these commented out lines meant to communicate? Are they meant to communicate something about the current contract? (i.e. protocolFee is 3, etc). Or are they meant to be placeholders for future functionality?

  • ah - I see that these are implemented inside of PoolState. can we remove them from poolv1 in this case? yes, I will do that, that was a gas optimization I did the updates last days according to issue opened by Miao https://github.com/donoso-eth/spool-foundry/issues/1 and wanted to test before deleting them 👉 cleaned

• closeAccount() in PoolInternal has no implementation. What is the purpose of that function? 👉 Already implemented, close streams and withdraw all in one transaction.

• What are the emergency functions used for? 👉 the emergency functions should pause the protocol in the case that one incident happen and provide helper functions to stop all streams and rebalance the accounts if required, these functions will be called off chain

• What is readStorageSlot() used for? 👉 cleaned, used another approach for testing storage

PoolInternal-V1 Contract

• On what intervals will we recommend that _balanceTreasuryFromGelato() be called? My understanding is that this is called at a pre-defined interval. We’ll use every state change as an opportunity to balance the entire treasury, regardless of who calls that state-changing operation. However, this function is designed to reduce reliance on regular interaction from users for treasury to be balanced. Is this correct? 👉 👉 Gelato will check every block whether the balanceTreasury() has to be fired or not, it does this with a resolver contract that checks the following:

   canExec =
          block.timestamp >= lastExecution + BALANCE_TRIGGER_TIME && (pool.outFlowRate > 0 
          || //or
          poolBalance > DEPOSIT_TRIGGER_AMOUNT);

  if canExec returns true, then will be executed and this happens in two scenarios

1. The pool is outstreaming (pool.outFlowRate > 0) and the time elapsed since the last execution is greater than BALANCE_TRIGGER_TIME (24h), then we need to rebalance the pool to ensure remains with enough liquidity for the next 24 hours.
2. If the pool superToken balance is greater than a certain threshold the balance treasury task will push this amount to our strategy ensuring max capital efficiency.

• Lines 365 and 366 are commented out, is there a reason for this? 👉 cleaned

cc @kobuta23

[donoso-eth]

Your comments are incredibly valuable, will change accordingly! 🫡 Answers are inlined

[donoso-eth]

all comments done, only still to be done:

Can we use the new SuperTokenV1Library instead of CFALibrary? Will save a lot of boilerplate and be much more readable. I can help with this and you can find the library here