Fix/issues 298 299#356
Merged
Merged
Conversation
… and document TotalDeposits/TotalAssets relationship Issue Neurowealth#298 – verify-deployment.sh smoke checks: - deploy-devnet.sh now writes OWNER_ADDRESS to devnet-contracts.env so that verify-deployment.sh can run end-to-end without a missing-variable error. The deployer is the initial owner after initialize(), so OWNER_ADDRESS is set equal to DEPLOYER_ADDRESS in devnet environments. Issue Neurowealth#299 – reconcile TotalDeposits with share-based TotalAssets after yield: - Document the design decision explicitly: TotalDeposits is intentionally not synced on yield updates; it is a principal-only counter. All cap guards and share-pricing use TotalAssets (already correct in code). - Update lib.rs module-level storage doc and get_total_deposits() doc comment with a relationship table and a cross-reference to ARCHITECTURE.md. - Update ARCHITECTURE.md section to reference issue Neurowealth#299, add the design-decision note, and link to the regression tests. - Update test_total_assets_cap.rs module doc to reference Neurowealth#299 and state the design rationale inline for readers of the test file.
|
@Mrwicks00 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
Author
|
@Abidoyesimze check |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses two open issues in a single focused PR.
Issue #298 —
scripts/verify-deployment.shfor post-deploy smoke checksverify-deployment.shalready exists and satisfies all acceptance criteria(reads
devnet-contracts.env, checksget_version/init state,get_agent,get_usdc_token,is_paused, exits non-zero on failure).Gap fixed:
deploy-devnet.shwas writingDEPLOYER_ADDRESSto the envfile but not
OWNER_ADDRESS, whichverify-deployment.shrequires as amandatory variable. After
initialize()the deployer is the owner, so:The full flow
./scripts/deploy-devnet.sh && ./scripts/verify-deployment.shnow runs without a
missing required variable: OWNER_ADDRESSerror.Issue #299 — Reconcile
TotalDepositswith share-basedTotalAssetsafter yieldDesign decision (already correct in code, now documented explicitly):
TotalDepositsis intentionally not synced whenupdate_total_assets()iscalled. It is a principal-only counter useful for off-chain reporting. All
on-chain economic calculations — share minting, user redemption, TVL cap
enforcement — use
TotalAssets(principal + yield).TotalDepositsdeposit,withdrawTotalAssetsdeposit,withdraw,update_total_assetsChanges:
neurowealth-vault/contracts/vault/src/lib.rs: updated module-level storagetable and
get_total_deposits()doc comment with a relationship table and across-reference to
ARCHITECTURE.md.ARCHITECTURE.md: §"TotalDeposits vs TotalAssets" now references issue Reconcile TotalDeposits with share-based TotalAssets after yield #299,states the design decision, and links to the regression test file.
tests/test_total_assets_cap.rs: module doc updated to reference issue Reconcile TotalDeposits with share-based TotalAssets after yield #299and state the design rationale inline.
CHANGELOG.md: entries added under[Unreleased].Regression coverage (all 4 tests pass):
test_tvl_cap_blocks_deposit_after_yield_accrual— cap check rejects depositsonce TotalAssets (not TotalDeposits) exceeds the cap.
test_deposit_accepted_when_total_assets_plus_amount_within_cap— depositaccepted while TotalAssets remains below cap after yield.
test_deposit_yield_withdraw_cap_regression— full lifecycle: deposit → yield→ withdraw → cap check.
test_total_assets_reflects_yield_while_total_deposits_stays_as_principal—confirms TotalDeposits is unchanged by yield while TotalAssets grows.
Checklist
CHANGELOG.mdunder[Unreleased]for any contract behavior, event, or error-code changes.get_version(), the new version number is noted inCHANGELOG.md. (no version bump — documentation/script only)Versionstorage value. (N/A)EVENTS.md. (no new events)Closes #298
Closes #299