fix: reduce storage overhead and fix contract inconsistencies - #1
Open
confima-source wants to merge 15 commits into
Open
fix: reduce storage overhead and fix contract inconsistencies#1confima-source wants to merge 15 commits into
confima-source wants to merge 15 commits into
Conversation
…/docs/issue-278-vault-entry-field-docs docs: add unit documentation to VaultEntry and LedgerVaultEntry field…
…x/multiple-issues-337-339-341-343 fix: resolve issues decentralized-time-lock-vault#337, decentralized-time-lock-vault#339, decentralized-time-lock-vault#341, decentralized-time-lock-vault#343
- Storage overhead:
- get_deposit_ids: replace O(counter) full scan with O(k) ActiveDepositIds key
- add_depositor: replace O(n) list scan with O(1) DepositorMember key check
- contract.rs: remove duplicate imports (constants now in constants.rs only)
- All events now include deposit_id in their data payload
- Contract inconsistencies:
- deposit/withdraw/emergency_withdraw events: add deposit_id to data
- cancel_deposit: fix inverted guard — return FundsAlreadyUnlocked (code 13)
when now >= unlock_time instead of misusing FundsStillLocked
- batch_emergency_withdraw: implement missing function with BatchTooLarge (code 14) guard
- withdraw_to, paused, unpaused: add missing event helpers in events.rs
- New error codes:
- FundsAlreadyUnlocked = 13
- BatchTooLarge = 14
- Tests:
- batch_emergency_withdraw: all succeed, skip missing, non-admin fails, too large
- get_deposit_ids: active-only tracking after partial withdrawal
- add_depositor: no duplicate added for multiple deposits from same address
- FundsAlreadyUnlocked: cancel after unlock and at exact unlock time
…time-lock-vault#274, decentralized-time-lock-vault#275, decentralized-time-lock-vault#336 - docs(changelog): document ledger-based deposit timing semantics for deposit and deposit_for, including unlock_time validation rules and usage examples (decentralized-time-lock-vault#273) - docs(contributing): add Soroban-specific testing guidance covering testutils environment, ledger time manipulation, test conventions, and SDK version policy (decentralized-time-lock-vault#274) - docs(security): add responsible disclosure timeline table and four-level severity guidelines (Critical/High/Medium/Low) (decentralized-time-lock-vault#275) - ci: add test-release job running cargo test --release --features testutils to catch optimisation-related regressions; build job now depends on test-release (decentralized-time-lock-vault#336) Closes decentralized-time-lock-vault#273 Closes decentralized-time-lock-vault#274 Closes decentralized-time-lock-vault#275 Closes decentralized-time-lock-vault#336
…t to initialize - types.rs: replace DepositorList (Vec) with DepositorCount + DepositorAt(u32) + DepositorIndex(Address) keys for O(1) add/remove - storage.rs: rewrite add/remove_depositor using swap-remove; eliminate linear scan and growing single-key Vec blob - contract.rs: cap get_depositors limit at 100 to prevent unbounded page reads; add fee_recipient: Option<Address> param to initialize so penalty destination is set at deployment time - test.rs: update all setup helpers for new initialize signature; fix test_cancel_deposit_partial_penalty_splits_correctly for real fee_recipient routing; add 8 new tests covering limit cap, zero limit, swap-remove correctness, cancel removes depositor, fee_recipient via initialize, penalty routing, and no-fee-recipient initialisation
…am/fix/issues-273-274-275-336 fix: resolve docs and CI issues decentralized-time-lock-vault#273, decentralized-time-lock-vault#274, decentralized-time-lock-vault#275, decentralized-time-lock-vault#336
…x/issues-340-342-344-346 ci: add Cargo ecosystem to Dependabot with soroban group
…2-code/fix/storage-perf-and-correctness fix: O(1) depositor index, cap get_depositors limit, add fee_recipien…
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
5 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
Fixes two classes of issues: unnecessary storage/compute overhead and contract behavior inconsistencies.
Storage / Compute Overhead
get_deposit_idsO(n) → O(k): replaced the full0..counterscan with a newVaultKey::ActiveDepositIdskey that tracks only live deposit IDs.add_depositorO(n) → O(1): replaced the full list scan with aVaultKey::DepositorMemberboolean key for O(1) membership check.constants.rs.Contract Inconsistencies
deposit_id:deposit,withdraw, andemergency_withdrawevents now includedeposit_idin their data payload.cancel_depositinverted guard: returns newFundsAlreadyUnlockederror (code 13) whennow >= unlock_timeinstead of misusingFundsStillLocked.batch_emergency_withdrawimplemented: best-effort batch withBatchTooLargeguard (code 14).withdraw_to,paused,unpaused.New Error Codes:
FundsAlreadyUnlocked(13),BatchTooLarge(14)Tests Added
batch_emergency_withdraw: all succeed, skip missing, non-admin fails, too-large failsget_deposit_ids: active-only tracking after partial withdrawaladd_depositor: no duplicate in list for multiple deposits from same addressFundsAlreadyUnlocked: cancel after unlock and at exact unlock time