Security: Eliminate Double-Counting Vulnerability Class via Receipt-Based Accounting

[kgrgpg]

Security: Eliminate Double-Counting Vulnerability Class via Receipt-Based Accounting

Closes: #N/A (Proactive security hardening based on audit findings)

Description

This PR introduces a fundamental architectural refactoring of the FlowCreditMarket lending protocol to use receipt-based accounting instead of mutable ledger-based accounting. This change leverages Cadence's resource-oriented programming model to prevent double-counting bugs at the type system level, ensuring this class of vulnerability cannot recur.

The Problem: Why the Audit Bug Could Happen Again

During the external audit, a critical double-counting bug was identified in the deposit flow. The issue was fixed in commit 3c3272a ("Math adjustments from external review"), but the underlying architectural pattern that allowed this bug to exist remains a systemic risk.

The Original Bug (commit 3c3272a):

// BEFORE (buggy):
let depositAmount = from.balance           // Capture balance BEFORE potential split
let uintDepositAmount = FlowCreditMarketMath.toUFix128(depositAmount)
...
// Vault may be split here for deposit limiting, reducing from.balance
...
position.balances[type]!.recordDeposit(amount: uintDepositAmount, tokenState: tokenState)
// Records the ORIGINAL amount, not the actual deposited amount

// AFTER (quick fix):
let depositAmount = from.balance
...
// Vault may be split here
...
position.balances[type]!.recordDeposit(amount: FlowCreditMarketMath.toUFix128(from.balance), tokenState: tokenState)
// Now records from.balance at the time of recording

Why this class of bug can recur with the current architecture:

The root cause is the architectural pattern of maintaining a separate mutable ledger (InternalBalance struct with recordDeposit/recordWithdrawal methods) that must be manually kept in sync with actual token movements. This pattern:

1. Requires developers to remember to update the ledger at the correct point in time
2. Allows the ledger state to diverge from actual token holdings
3. Creates multiple code paths where synchronization can fail
4. Does not leverage Cadence's resource safety guarantees

The Solution: Receipt-Based Accounting

Instead of maintaining a mutable ledger, this PR introduces receipt resources as the single source of truth:

OLD PATTERN (Ledger-Based):
--------------------------
Tokens arrive --> Update mutable ledger --> Ledger IS the record
                  (manual, error-prone)

NEW PATTERN (Receipt-Based):
----------------------------
Tokens arrive --> Create receipt resource atomically --> Receipt IS the record
                  (enforced by type system)

Core Principle

A DepositReceipt or BorrowReceipt resource is created atomically when tokens are deposited or borrowed. The receipt:

1. Cannot be copied (it's a Cadence resource)
2. Must be destroyed to withdraw tokens
3. Is the record itself - there is no separate ledger to sync

New Resource Types

access(all) resource DepositReceipt {
    access(all) let poolUUID: UInt64
    access(all) let tokenType: Type
    access(all) let scaledShares: UFix128  // Immutable once created
    access(all) let createdAt: UFix64
    
    access(all) view fun trueBalance(creditIndex: UFix128): UFix128 {
        return FlowCreditMarket.scaledBalanceToTrueBalance(self.scaledShares, interestIndex: creditIndex)
    }
}

access(all) resource BorrowReceipt {
    access(all) let poolUUID: UInt64
    access(all) let tokenType: Type
    access(all) let scaledShares: UFix128  // Immutable once created
    access(all) let createdAt: UFix64
    
    access(all) view fun trueDebt(debitIndex: UFix128): UFix128 {
        return FlowCreditMarket.scaledBalanceToTrueBalance(self.scaledShares, interestIndex: debitIndex)
    }
}

Why This Makes Double-Counting Impossible

With receipts, the deposit flow becomes:

// Receipt-based deposit (simplified):
fun depositAndPush(..., from: @{FungibleToken.Vault}, ...) {
    // 1. Split vault if needed for deposit limits
    let actualVault <- splitIfNeeded(<-from)
    
    // 2. Calculate shares based on ACTUAL vault balance (cannot be wrong)
    let shares = calculateShares(actualVault.balance)
    
    // 3. Create receipt ATOMICALLY - the receipt IS the record
    let receipt <- create DepositReceipt(shares: shares, ...)
    
    // 4. Store receipt and deposit tokens together
    position.depositReceipts[type] <-! receipt
    reserves.deposit(from: <-actualVault)
}

It is impossible to record the wrong amount because:

• The receipt is created from the actual vault being deposited
• You cannot create a receipt without having the tokens
• You cannot access the tokens without destroying the receipt

Position Structure Change

// OLD: Mutable ledger that can get out of sync
access(all) resource InternalPosition {
    var balances: {Type: InternalBalance}  // Mutable struct with recordDeposit/recordWithdrawal
    ...
}

// NEW: Receipt resources ARE the record
access(all) resource InternalPosition {
    var depositReceipts: @{Type: DepositReceipt}   // Resources, not mutable structs
    var borrowReceipts: @{Type: BorrowReceipt}     // Cannot be copied or faked
    ...
}

Changes Overview

Removed (Legacy Patterns)

• InternalBalance struct with mutable direction and scaledBalance fields
• InternalBalance.recordDeposit() method
• InternalBalance.recordWithdrawal() method
• InternalPosition.balances field

Added (Receipt-Based Patterns)

• DepositReceipt resource
• BorrowReceipt resource
• InternalPosition.depositReceipts resource dictionary
• InternalPosition.borrowReceipts resource dictionary
• Helper functions: getDepositShares(), getBorrowShares(), getDepositTypes(), getBorrowTypes()

Updated Functions

Function	Change
depositAndPush	Creates DepositReceipt atomically with token deposit
withdrawAndPull	Consumes DepositReceipt atomically with token withdrawal
liquidate	Uses receipts for balance adjustments
healthFactor	Derives from PositionView.creditBalances/debitBalances
maxWithdraw	Uses new PositionView structure
buildPositionView	Populates from receipts, not mutable ledger
quoteLiquidation	Uses new PositionView structure

View Layer Refactoring

The PositionView struct was refactored to align with the receipt model:

// OLD:
struct PositionView {
    let balances: {Type: InternalBalance}  // Single dict with direction
    ...
}

// NEW:
struct PositionView {
    let creditBalances: {Type: UFix128}  // Derived from DepositReceipts
    let debitBalances: {Type: UFix128}   // Derived from BorrowReceipts
    ...
}

Testing

All 89 tests pass:

• FlowCreditMarket submodule: 45 tests
• Main tidal-sc repository: 44 tests

Benefits

1. Eliminates double-counting bugs by design - Cannot record deposits without having the tokens
2. Leverages Cadence's type system - Resource linearity enforced by compiler
3. Simpler mental model - Receipt IS the record, no separate ledger to sync
4. Auditability - Receipt creation/destruction provides clear audit trail
5. Future-proof - Pattern resistant to future code changes introducing similar bugs

Backward Compatibility

This is a breaking change to the internal data structures. External APIs (PositionDetails, PositionBalance) remain unchanged.

Critical Files to Review

• cadence/contracts/FlowCreditMarket.cdc - Core contract changes
• cadence/tests/phase0_pure_math_test.cdc - Updated tests for new PositionView structure

References

• Audit fix commit: 3c3272a ("Math adjustments from external review")
• Cadence resource documentation: https://cadence-lang.org/docs/language/resources

---

This refactoring ensures that the class of double-counting bugs identified in the audit cannot occur again, as the accounting model now leverages Cadence's resource safety guarantees rather than relying on manual ledger synchronization.

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the standards mentioned here.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[kgrgpg]

Calling review from:

@Kay-Zee @sisyphusSmiling @dete @nialexsan