TidalProtocol.cdc: implement dete review feedback (configurable insurance/limits; remove capacity growth; safety/asserts; event types; logging guard; health APIs)

[kgrgpg]

This PR addresses feedback from dete’s review (see PR #44: #44). Full triage and mapping are in PR_REVIEW_TRIAGE_dete.md in the repo.

Implemented

• Events and Types: use vaultType: Type in Deposited/Withdrawn; emit sites updated.
• Safety/Correctness: replace silent clamp-to-zero with explicit underflow assertions in credit/debit balance updates; harmonize recordDeposit comparison to >=; refine post-withdrawal health assertion to skip only when a top-up was used in the same call and immediate health is 0 (transitional).
• Configurability: add per-token insuranceRate (default 0.001) with governance setter; add per-token depositLimitFraction (default 0.05) with governance setter; updateInterestRates() uses insuranceRate; depositLimit() = capacity * depositLimitFraction.
• Capacity: remove unused deposit-capacity growth (capacity is fixed at cap; growth never executed); retain per-deposit limiting with clear rationale.
• Logging: add debugLogging flag and wrap verbose logs (default true).
• API and Docs: implement get/set for {min,target,max} health with validation via capability-safe pool methods; move BalanceDirection near core types; add design notes on where/why 128-bit fixed-point is used vs UFix64 at boundaries; clarify comments for negative credit rate handling and deposit-limit rationale.
• Tests: ./run_tests.sh passes across the suite.

Deferred / needs clarification (see PR_REVIEW_TRIAGE_dete.md)

• Numeric types/ranges: switch recordDeposit/Withdrawal params to UFix64 and reduce UInt128 usage; evaluate future UFix128 migration when available. Added design notes now; type changes deliberately deferred.
• Duplicate health/effective computations: further unify all paths via pure helpers; unify top-up vs no-top-up logic into a single path parameterized by optional source.
• Optional follow-ups: governance-configurable debugLogging; script adapter to stringify event vaultType for off-chain viewing; make liquidation bonus configurable (discussion pending).

Related Work: Liquidation mechanism branches

• Liquidation PRs (Liquidation Mechanism: Phases 1 & 2 (core + DEX) #41, Liquidation Phase 2 (DEX): stacked on #41 #43) add governance-configurable liquidation parameters (target HF, warm-up, protocol fees), per-token liquidationBonus, DEX guards, and execution/quoting paths. These map to several comments in PR DO NOT MERGE: A dummy PR to capture code review comments. #44 around liquidation bonuses, safety of liquidation flows, and explicit configurability.
• Our current PR aligns with that direction by making deposit/insurance parameters configurable and improving safety/asserts/logging. Post-merge, we intend to harmonize logging guards in liquidation paths and optionally standardize event type fields there as well.

Scope notes

• Off-chain consumers are out-of-scope for this PR; focus is the TidalProtocol contract.

Links

• Original review PR: DO NOT MERGE: A dummy PR to capture code review comments. #44
• Liquidation mechanism PRs: Liquidation Mechanism: Phases 1 & 2 (core + DEX) #41, Liquidation Phase 2 (DEX): stacked on #41 #43
• Triage mapping: PR_REVIEW_TRIAGE_dete.md

[codecov]

Codecov Report

❌ Patch coverage is 57.03125% with 55 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cadence/contracts/TidalProtocol.cdc	56.69%	55 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Kay-Zee]

This comment only makes sense in the context of this PR. We should not have these types of comments in the code. If we're trying to add context for the reviewer, let's do that on GH PR, and not in SC comments

[Kay-Zee]

if default true here, i don't think it changes much. I think if this flag is how we're moving forward with debug logging, it should probably not be true by default.

[Kay-Zee]

so many unnecessary checks no? I feel like re-checking self.debug on every line is not providing anything.

[kgrgpg]

Addressed review feedback on TidalProtocol.cdc:

• Removed PR-specific inline comments; rephrased to neutral.
• Set Pool.debugLogging default to false.
• Consolidated repeated debugLogging checks in withdraw failure logging under a single guarded block.
• Kept functional logs; they emit only when debugLogging is true.

Changes pushed to review/dete-comments-audit. Please re-review.

[Kay-Zee]

let's fix this issue similarly to how you fixed the other debug logs

[kgrgpg]

Consolidated at other places

[Kay-Zee]

Currently, the cadence/tests/position_lifecycle_happy_test.cdc test fails on this assert, due to usedTopUp being false, even though postHealth is 0. Can we rethink why this might be the case?

does it matter that we didn't use the top up source here? or should we have attempted to use it anyways (and i guess likely have gotten 0?)

[Kay-Zee]

The commit looks good, but can we double check why the coverage is so low? i would've expected most of the changes to have been covered by the existing tests

[kgrgpg]

Added more governance tests and debugging logs enabled to improve coverage from 32 to 57%