Skip to content

Update contract comments & reorganize for readability#12

Merged
sisyphusSmiling merged 14 commits into
gio/refactor-pool-creation-updatedfrom
gio/update-comments
Jun 21, 2025
Merged

Update contract comments & reorganize for readability#12
sisyphusSmiling merged 14 commits into
gio/refactor-pool-creation-updatedfrom
gio/update-comments

Conversation

@sisyphusSmiling

@sisyphusSmiling sisyphusSmiling commented Jun 17, 2025

Copy link
Copy Markdown
Contributor

Stacked on: #10

Description

  • Prepare contract for broader refactor
  • Add clarifying code comments
  • Restructure protocol contract code to follow the sequence of construct layering, helping readability
  • Minor refactoring to internal methods to reduce code duplication and increase formatting consistency
  • Remove unused constructs

@sisyphusSmiling sisyphusSmiling self-assigned this Jun 17, 2025
@sisyphusSmiling sisyphusSmiling added the chore has to get done & doesn't add functionality label Jun 17, 2025
Comment thread cadence/contracts/TidalProtocol.cdc Outdated
Comment on lines +1764 to +1782
/// Transforms the provided `scaledBalance` to a true balance
// TODO: Clarify what "scaled" and "true" balances signify
access(all) view fun scaledBalanceToTrueBalance(scaledBalance: UFix64, interestIndex: UInt64): UFix64 {
// The interest index is essentially a fixed point number with 16 decimal places, we convert
// it to a UFix64 by copying the byte representation, and then dividing by 10^8 (leaving and
// additional 10^8 as required for the UFix64 representation).
let indexMultiplier = UFix64.fromBigEndianBytes(interestIndex.toBigEndianBytes())! / 100000000.0
return scaledBalance * indexMultiplier
}

/// Transforms the provided `trueBalance` to a scaled balance
// TODO: Clarify what "scaled" and "true" balances signify
access(all) view fun trueBalanceToScaledBalance(trueBalance: UFix64, interestIndex: UInt64): UFix64 {
// The interest index is essentially a fixed point number with 16 decimal places, we convert
// it to a UFix64 by copying the byte representation, and then dividing by 10^8 (leaving and
// additional 10^8 as required for the UFix64 representation).
let indexMultiplier = UFix64.fromBigEndianBytes(interestIndex.toBigEndianBytes())! / 100000000.0
return trueBalance / indexMultiplier
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could use some clarification on the difference between and and intentions behind "scaled" and "true" balances @kgrgpg @dete

@kgrgpg kgrgpg Jun 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Scaled” and “true” (sometimes called “principal” and “accrued” or “actual”) balances are a common pattern in lending & interest-bearing protocols such as Compound and Aave.
The idea is to keep per-account state very small and avoid touching every account each time interest accrues.

  1. Interest is represented globally by an “index”
    • Think of the index as “how many actual tokens does 1 unit of scaled balance represent right now?”.

  2. Scaled balance
    • This is what is persisted in each user’s storage.
    • It is the user’s deposit divided by the index that existed the last time they interacted with the pool.
    • Because every depositor’s scaled balance is divided by the same index, the sum of all scaled balances stays constant even while interest accrues—so we don’t have to update every account each block.

  3. True balance (actual balance)
    • This is what users care about: the real amount they own including the interest that has built up since their last action.
    • It is obtained on-the-fly by multiplying the stored scaled balance by the current index.

Mathematically

trueBalance  =  scaledBalance × currentIndex
scaledBalance =  trueBalance  ÷ currentIndex

Why the two representations?

• Gas / storage efficiency – Updating a single UInt64 index each block is much cheaper than iterating over every depositor and mutating their balances.
• Constant-sum invariant – Because scaled balances don’t change when interest accrues, accounting is simpler (totalScaledSupply never changes, totalTrueSupply grows automatically as the index grows).
• Deterministic conversions – Anyone can derive the same true balance at any time from on-chain data (scaled balance + current index), so there is no loss of information.

Typical lifecycle

  1. User deposits 100 tokens when index = 1.0000 → scaledBalance = 100 / 1.0000 = 100.
  2. Time passes, index increases to 1.0500.
  3. When the user checks their balance: trueBalance = 100 × 1.0500 = 105 (100 principal + 5 interest).
  4. If they now withdraw 20: trueWithdrawalScaled = 20 / 1.0500 ≈ 19.0476; their stored scaledBalance is reduced by that amount; the contract transfers 20 real tokens.

Key take-aways for the docs:

• “Scaled balance” = balance expressed in units of the reserve’s index, stored on chain.
• “True balance” (or “actual balance”) = balance in underlying tokens the user can withdraw at the current index.
• The conversion factor is the reserve’s interestIndex: true = scaled × index.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How the interest index is produced
──────────────────────────────────

  1. Notation
    r = annual borrow rate expressed as a decimal (e.g. 0.05 for 5 %).
    t = time elapsed since the index was last updated, in seconds.
    SECONDS_PER_YEAR ≈ 31 536 000 (365 × 24 × 60 × 60).
    lastIndex = index the last time it was computed.
    newIndex = index after t seconds have passed.

  2. Continuous-time formulation used by most money-markets

    newIndex = lastIndex × (1 + r × t / SECONDS_PER_YEAR)

    This is exactly the simple-interest formula evaluated every block (or every explicit “accrue” transaction).
    Because the product r × t / SECONDS_PER_YEAR is normally a very small number (e.g. for r = 5 % and t = 1 s it is 1.586 × 10⁻⁹), compounding once per second deviates from pure simple interest by less than one billionth per step.

  3. Why the deviation from simple interest is negligible

    Suppose we instead compounded continuously (the textbook eʳᵗ model).
    Over one second:

     e^(r/SECONDS_PER_YEAR) ≈ 1 + r/SECONDS_PER_YEAR + ½(r/SECONDS_PER_YEAR)² + …
    

    The second-order term is about 1.26 × 10⁻¹⁷ for r = 5 %, so the difference between:
    • simple step: 1 + r/SECONDS_PER_YEAR
    • continuous compounding: e^(r/SECONDS_PER_YEAR)

    is on the order of 10⁻¹⁷ per second—far below the precision of UFix64 (10⁻⁸).
    Even after a full year, compounding every second versus pure simple interest differs by:

     (1 + r/SECONDS_PER_YEAR)^(SECONDS_PER_YEAR) − (1 + r) ≈ 0.05127 − 0.05 = 0.00127  
    

    i.e. 0.127 percentage-points, or ~2.5 % relative difference—still modest, but most protocols treat this extra as correct “compound” interest.

  4. Implementation sketch (pseudocode)

    fun accrueInterest(currentTime): // runs at most once per block
    t = currentTime - lastAccrualTimestamp
    if t == 0: return

     perSecondRate = borrowRate / SECONDS_PER_YEAR
     interestFactor = 1.0 + perSecondRate * t    // simple-interest step
     newIndex = lastIndex * interestFactor
    
     lastIndex = newIndex
     lastAccrualTimestamp = currentTime
    
  5. Effect of a fixed 5 % rate

    • Right after deployment: index = 1.0000 × 10¹⁶
    • After three months (≈ 7 889 400 s):

     interestFactor = 1 + 0.05 × 7 889 400 / 31 536 000 ≈ 1.0125  
     index = 1.0000 × 1.0125 = 1.0125 × 10¹⁶  
    

    A depositor’s scaled balance of 100 becomes a true balance of 101.25 tokens.

  6. Why update is infrequent but still accurate

    • Because the formula is linear in t (simple interest), we can accumulate any amount of time with a single multiplication.
    • Whether we call accrueInterest each block, once per hour, or only when someone interacts, the final index as a function of wall-clock time is identical (no missed interest), barring rounding.
    • Rounding error is limited to the 8-decimal precision of UFix64; with per-second steps the worst-case cumulative error over a year is < 0.00000001 tokens per unit deposited—economically irrelevant.

Key take-aways to add to the doc/comment

• The protocol uses a simple-interest-per-second model: index grows linearly with elapsed time and current borrow rate.
• Because the step size r × t / SECONDS_PER_YEAR is tiny, its behaviour is almost indistinguishable from continuous compounding on a per-block scale.
• Updating the global interestIndex once per interaction is sufficient; users’ balances stay correct up to UFix64 precision.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if users don't all deposit funds at the same time? Wouldn't you need a different index for each user since their interest started accruing at different times?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if users don't all deposit funds at the same time? Wouldn't you need a different index for each user since their interest started accruing at different times?

This is also my question. It looks like the interest index is currently calculated on the global TokenState per reserve type which would dictate the same interest index per position. But if when I deposit I expect index == 1.0 and the TokenState of my deposited token is > 1.0, then I've immediately accrued interest that shouldn't be due. Or am I misunderstanding something?

@joshuahannan joshuahannan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this needs some of the architectural changes we talked about and that I mentioned in my other comments, but I know y'all have to get this finished for the prototype so given the current design, it seems fine to me

Comment thread cadence/contracts/TidalProtocol.cdc Outdated
Comment thread cadence/contracts/TidalProtocol.cdc Outdated
Comment thread cadence/contracts/TidalProtocol.cdc Outdated
Comment on lines +249 to +251
/// The index of the credit interest for the related token
access(all) var creditInterestIndex: UInt64
/// The index of the debit interest for the related token

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by index of credit interest?

Comment on lines +1201 to +1209
// BUG: Calling through to withdrawAndPull results in an insufficient funds from the position's
// topUpSource. These funds should come from the protocol or reserves, not from the user's
// funds. To unblock here, we just mint MOET when a position is overcollateralized
// let sinkVault <- self.withdrawAndPull(
// pid: pid,
// type: sinkType,
// amount: sinkAmount,
// pullFromTopUpSource: false
// )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused here. I thought if it is overcollatoralized, the user's funds since their funds are the ones that caused it to be overcollatoralized. So they should just get their funds back, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the position is overcollateralized, the protocol should direct the overflown value back to the user (if they have a drawDownSink configured). But the protocol should direct that value as MOET - the protocol stablecoin - which is the only denomination the drawDownSink can accept.

I'm not sure why the withdrawAndPull method was originally called here based on this PR comment thread. I think it was to pull MOET from Pool reserves, but I think the protocol should instead mint MOET to the drawDownSink.

///
/// A Position is an external object representing ownership of value deposited to the protocol. From a Position, an
/// actor can deposit and withdraw funds as well as construct DeFiBlocks components enabling value flows in and out
/// of the Position from within the context of DeFiBlocks stacks.
// TODO: Consider making this a resource given how critical it is to accessing a loan

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're still going to do this, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I still think this needs to be a resource and changes here will be a part of refactor post-prototype.

Comment thread cadence/contracts/TidalProtocol.cdc Outdated
Comment on lines +1764 to +1782
/// Transforms the provided `scaledBalance` to a true balance
// TODO: Clarify what "scaled" and "true" balances signify
access(all) view fun scaledBalanceToTrueBalance(scaledBalance: UFix64, interestIndex: UInt64): UFix64 {
// The interest index is essentially a fixed point number with 16 decimal places, we convert
// it to a UFix64 by copying the byte representation, and then dividing by 10^8 (leaving and
// additional 10^8 as required for the UFix64 representation).
let indexMultiplier = UFix64.fromBigEndianBytes(interestIndex.toBigEndianBytes())! / 100000000.0
return scaledBalance * indexMultiplier
}

/// Transforms the provided `trueBalance` to a scaled balance
// TODO: Clarify what "scaled" and "true" balances signify
access(all) view fun trueBalanceToScaledBalance(trueBalance: UFix64, interestIndex: UInt64): UFix64 {
// The interest index is essentially a fixed point number with 16 decimal places, we convert
// it to a UFix64 by copying the byte representation, and then dividing by 10^8 (leaving and
// additional 10^8 as required for the UFix64 representation).
let indexMultiplier = UFix64.fromBigEndianBytes(interestIndex.toBigEndianBytes())! / 100000000.0
return trueBalance / indexMultiplier
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if users don't all deposit funds at the same time? Wouldn't you need a different index for each user since their interest started accruing at different times?

@sisyphusSmiling

Copy link
Copy Markdown
Contributor Author

Merging post-approval, though looks like there are still open questions about interest calculation

@sisyphusSmiling
sisyphusSmiling merged commit 9d788ec into gio/refactor-pool-creation-updated Jun 21, 2025
1 check passed
@sisyphusSmiling
sisyphusSmiling deleted the gio/update-comments branch June 21, 2025 00:41
sisyphusSmiling added a commit that referenced this pull request Jun 25, 2025
* feat: Restore AlpenFlow implementation with supporting contracts

- TidalProtocol: 100% restoration of Dieter's AlpenFlow functionality
  - Oracle-based pricing and health calculations
  - Deposit rate limiting and position queues
  - Advanced health management functions
  - Async position updates
- MOET: Mock stablecoin for multi-token testing
- TidalPoolGovernance: Role-based governance system
- AlpenFlow_dete_original: Reference implementation

Note: Old tests removed as they're incompatible with new contracts.
Updated tests coming in follow-up PR.

No breaking changes. Foundation for multi-token lending protocol.

* fix: Add MOET and TidalPoolGovernance to flow.json for deployment

* remove DeFiBlocks interface definitions from TidalProtocol contract

* update InternalPosition.topUpSource to value field from reference

* fix PriceOracle conformance .price() calls

* Update cadence/contracts/TidalProtocol.cdc

* Assign TokenState.lastUpdate as current block timestamp on init

* update error message

* remove TidalGovernance contract

* add select contract, tranasction, script, and test changes from #6

* update DeFiBlocks submodule to latest main

* add new DFB dependencies to flow.json

* fix PositionSink/Source conflict with IdentifiableStruct.id()

* update PositionSource fields from access(all) to access(self)

* add check on FungibleToken defining contract conformance when token added

* remove unused methods from test_helpers.cdc

* remove redundant TidalProtocol position sink/source definitions

* remove comment notes

* fix insufficient funds on rebalance error by minting MOET instead of pulling from topUpSource

* remove contract debug logs

* add behavioral test cases for position creation & under/overcollateralization rebalancing

* remove unused methods, structs & implement TODOs

* protects against underflow on TokenState.updateCredit/DebitBalance

* restrict access on Position's privileged methods

* update minor contract formatting

* update DeFiBlocks with latest changes merged to main

* update DeFiBlocks to latest

* fix MOET deposit logic

* fix import syntax

* add initial protocol events (#13)

* update position rebalance transaction comments

* fix fundsAvailableAboveTargetHealthAfterDepositing when effectiveDebtAfterDeposit == 0.0

* Update contract comments & reorganize for readability (#12)

* update PositionDetails and PositionBalance comments and fields

* consolidate public methods to the bottom of the contract

* remove comment references to restored functionality

* update contract comments

* update contract comments and reorganize for clarity

* update contract and construct internal methods

* update contract comments

* revert changes to PositionDetails balances field

* update contract comments

* Update cadence/contracts/TidalProtocol.cdc

Co-authored-by: Joshua Hannan <joshua.hannan@flowfoundation.org>

* update interest index & balance type comments

---------

Co-authored-by: Joshua Hannan <joshua.hannan@flowfoundation.org>

* Update .gitmodules

---------

Co-authored-by: kgrgpg <keshav.pg@gmail.com>
Co-authored-by: Joshua Hannan <joshua.hannan@flowfoundation.org>
Co-authored-by: Alex <12097569+nialexsan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore has to get done & doesn't add functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants