Skip to content

chore(vault): make pre-initialization access consistently return NotInitialized - #400

Open
mac-dubem wants to merge 2 commits into
drydocs:mainfrom
mac-dubem:fix/uninitialized-access-return-notinitialized
Open

chore(vault): make pre-initialization access consistently return NotInitialized#400
mac-dubem wants to merge 2 commits into
drydocs:mainfrom
mac-dubem:fix/uninitialized-access-return-notinitialized

Conversation

@mac-dubem

Copy link
Copy Markdown

Summary

  • Changed get_admin, get_adapter, and require_admin to return ContractError::NotInitialized instead of panicking with .unwrap() when called before initialize()
  • Updated set_paused, set_admin, set_adapter, deposit, withdraw, and get_total_assets to propagate the NotInitialized error
  • Added 8 new tests covering the uninitialized case for each affected function

This makes error handling consistent — callers and off-chain indexers can match on ContractError::NotInitialized instead of special-casing which functions panic.

Test plan

  • cargo clippy --all-targets -- -D warnings passes
  • cargo test passes (32 vault tests, all passing)

Closes #375

@vercel

vercel Bot commented Jul 17, 2026

Copy link
Copy Markdown

@mac-dubem is attempting to deploy a commit to the Collins' projects Team on Vercel.

A member of the Team first needs to authorize it.

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified independently: checked this out, ran cargo clippy --all-targets -- -D warnings (clean) and cargo test -p meridian-vault --lib (32 passed, matches the PR's own claim). The ?-propagation is threaded correctly through every caller, and existing non-try_ test call sites still compile since Soroban's generated client wrapper auto-unwraps Result and panics on Err. Logic is correct.

Two things need fixing before merge.

PR Title check is failing. The title kept the bracketed issue-style format instead of conventional-commit format (type(scope): description). Since this repo squashes on merge, the PR title becomes the final commit message.

This branch predates #397 by about 33 minutes, which gitignored packages/contracts/**/test_snapshots/ and removed all tracked snapshot files. This PR still has 54 tracked snapshot files, including 10 new ones for the tests it adds. Please rebase onto current main and drop all of them from the diff, they're gitignored now and shouldn't be committed.

@collinsezedike

Copy link
Copy Markdown
Collaborator

Hey @mac-dubem, checking in on this one since it's been a few days quiet.

Are you still good to continue?

If you're stuck on anything, let me know here and I can help.

If you're no longer able to finish it, just say so and I'll pass the issue on to someone else.

…nitialized

Change get_admin, get_adapter, require_admin, set_paused, set_admin,
set_adapter, deposit, withdraw, and get_total_assets to return
ContractError::NotInitialized instead of panicking with .unwrap()
when called before initialize(). Add 8 tests covering the uninitialized
case for each affected function.

Drop tracked test_snapshots (already gitignored after drydocs#397).

Closes drydocs#375
@mac-dubem
mac-dubem force-pushed the fix/uninitialized-access-return-notinitialized branch from ae61964 to 9a21977 Compare July 25, 2026 10:32
@mac-dubem mac-dubem changed the title [Chore] Make pre-initialization access consistently return NotInitialized instead of panicking chore(vault): make pre-initialization access consistently return NotInitialized Jul 25, 2026
collinsezedike

This comment was marked as outdated.

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is failing on Commit Messages: the header is 79 characters, over the 72-char limit. Since this repo squash-merges using the PR title, both the commit header and the PR title need to be trimmed to 72 characters or under, e.g. chore(vault): return NotInitialized instead of panicking pre-init.

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.

[Chore] Make pre-initialization access consistently return NotInitialized instead of panicking

2 participants