Skip to content

Commit faf2982

Browse files
authored
Merge pull request #35 from onflow/refactor/better-code
refactor: implement phase 0 - extract pure math functions from positionHealth
2 parents 2d4ecb6 + 6b21665 commit faf2982

4 files changed

Lines changed: 855 additions & 19 deletions

File tree

.github/pr_bodies/PR_36.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Phase 0: availableBalance refactor to pure helpers + tests
2+
3+
This PR is stacked on top of [PR #35](https://github.com/onflow/TidalProtocol/pull/35) (base: `refactor/better-code`). It completes the Phase 0 vertical slice for health/withdraw by wiring the public query path to pure helpers while preserving existing top-up behavior.
4+
5+
## Summary
6+
- **availableBalance**:
7+
- Preserves deposit-assisted path when `pullFromTopUpSource=true` by delegating to `fundsAvailableAboveTargetHealthAfterDepositing(...)`.
8+
- Otherwise uses the new pure pipeline: `buildPositionView(pid)` + `maxWithdraw(...)` with 18-decimal `UInt256` math.
9+
- **Pure math tests**: Adds `cadence/tests/phase0_pure_math_test.cdc` covering `healthFactor` and `maxWithdraw` (both debt-increase and collateral-drawdown paths). Expectations computed in `UInt256` to match on-chain truncation.
10+
- **Docs**: Regenerates `REFACTORING_PLAN.md` code sections to use current Cadence syntax and mirror the implemented Phase 0 shape (incl. top-up path note).
11+
- **Housekeeping**: Removes deprecated `DeFiBlocks/` and `REFACTOR_EVALUATION.md` (superseded by DeFiActions and current plan).
12+
13+
## Rationale
14+
- Introduces a clean functional-core / imperative-shell boundary without changing end-user semantics.
15+
- Greatly improves testability of the core math (`healthFactor`, `maxWithdraw`).
16+
- Keeps deposit-assisted behavior intact for platforms relying on `topUpSource`.
17+
18+
## Implementation Notes
19+
- `buildPositionView(pid)` creates immutable snapshots per token (price, indices, risk) and copies of position balances; no storage mutation.
20+
- `maxWithdraw(view, withdrawSnap, withdrawBal, targetHealth)` solves the linear constraint to maintain target health; handles both credit and debit cases.
21+
- `availableBalance` now chooses:
22+
- Top-up path → original async/deposit-assisted logic.
23+
- Pure path → view + helper + `uint256ToUFix64(...)`.
24+
- Rounding is explicit in 18-decimal `UInt256` arithmetic to match chain truncation.
25+
26+
## Tests
27+
- Added: `cadence/tests/phase0_pure_math_test.cdc`
28+
- `test_healthFactor_zeroBalances_returnsZero`
29+
- `test_healthFactor_simpleCollateralAndDebt`
30+
- `test_maxWithdraw_increasesDebtWhenNoCredit`
31+
- `test_maxWithdraw_fromCollateralLimitedByHealth`
32+
- All existing suites remain passing with `run_tests.sh`.
33+
34+
## Backward Compatibility
35+
- No change to deposit-assisted semantics when `pullFromTopUpSource=true`.
36+
- Query-only behavior updated to use pure math; results validated against existing suites.
37+
38+
## Documentation
39+
- `REFACTORING_PLAN.md` snippets updated to current Cadence (`access(all)`, correct entitlements/types) and reflect the preserved top-up path in `availableBalance`.
40+
41+
## Next Phases (follow-ups)
42+
- Phase 1: introduce `applyWithdraw`/`applyDeposit`/`applyAccrual`/`applyLiquidation` with pre/post invariants, using the same pure helpers for validation before mutation.
43+
- Phase 2+: rate/capacity pure math; async queue refactor; governance resource & snapshots.
44+
45+
## How to verify locally
46+
```bash
47+
./run_tests.sh
48+
```
49+
All tests should pass, including the new Phase 0 tests.

0 commit comments

Comments
 (0)