Skip to content

Update lending pool fuzz target for LP-share model#41

Open
K1NGD4VID wants to merge 2 commits into
LabsCrypt:mainfrom
K1NGD4VID:fuzz_target_1
Open

Update lending pool fuzz target for LP-share model#41
K1NGD4VID wants to merge 2 commits into
LabsCrypt:mainfrom
K1NGD4VID:fuzz_target_1

Conversation

@K1NGD4VID

Copy link
Copy Markdown
Contributor

Changes Made

  • Removed the empty boilerplate fuzz target file fuzz/fuzz_targets/fuzz_target_1.rs and its binary configuration in fuzz/Cargo.toml.
  • Redesigned the invariants inside fuzz/fuzz_targets/lending_pool_fuzz.rs to conform to the share-based model of the contract instead of the stale 1:1 asset-to-share model.
  • Updated deposit and withdraw invocations to match current contract signatures (adding the token_id parameter).
  • Treated withdraw amount parameters explicitly as share count and renamed local variables (e.g. shares_to_withdraw).
  • Added comments explicitly indicating that withdraw parameters are share counts.
  • Wrapped fuzz actions in panic::catch_unwind to catch and report contract panics as fuzz crashes.
  • Added a sanity check test suite inside lending_pool/src/test.rs that executes the new invariants against a mock LendingPool, passing verification successfully.

Closes #21

@ogazboiz ogazboiz 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.

thanks for updating the fuzz target, but this can't merge as-is, it would destructively revert main. two hard blockers plus fmt:

  1. silent clobber. this branch is 21 commits behind main and carries an OLDER lending_pool/src/lib.rs that DELETES the managed-assets model now on main: it removes TotalManagedAssets, record_yield, set_loan_manager/get_loan_manager, get_total_managed_assets, WithdrawalCooldownActive, reverts CURRENT_VERSION 4->3, and reverts share pricing from total_managed_assets back to raw read_pool_balance. merging would silently wipe that with no conflict marker. please rebase: git fetch origin && git rebase origin/main (drop the stale lending_pool changes so the diff is fuzz-only), then re-run CI.

  2. it also bundles an older copy of your #39 remittance_nft consolidation (commit 0a46b75), which both duplicates #39 and re-introduces the trailing-whitespace fmt issues #39 already fixed. after rebasing, drop the remittance_nft commit and keep #41 fuzz-only, #39 is the canonical version of that change.

  3. fmt fails: lending_pool/src/test.rs:1520,1541,1548,1560,1573 and remittance_nft/src/lib.rs:608 + test.rs:1901. run cargo fmt --all from the repo root.

one semantic note for after the rebase: the fuzz invariant assert_no_value_creation uses raw token pool_balance as the redeemable basis, but on main share value derives from TotalManagedAssets, so the invariant may be stale against the current model, worth a look once it's rebased.

rebase to fuzz-only + fmt and i'll re-review.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Testing] fuzz_target_1 is an empty stub and lending_pool_fuzz asserts the obsolete 1:1 deposit invariant

2 participants