Skip to content

fix(vault): remove storage mutation from view getters - #1220

Merged
collinsezedike merged 1 commit into
drydocs:mainfrom
Mamavee001:fix/vault-views-storage-write
Oct 25, 2026
Merged

collinsezedike merged 1 commit into
drydocs:mainfrom
Mamavee001:fix/vault-views-storage-write

Conversation

@Mamavee001

@Mamavee001 Mamavee001 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Make get_entry_time and get_principal true read-only views by removing the clear_position_records storage write when an address has zero position balance. Update the architecture docs.

Closes #1041

@vercel

vercel Bot commented Oct 20, 2026

Copy link
Copy Markdown

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

@Mamavee001 the change itself is right, but three things need fixing. Soroban Contract Tests fails on cargo fmt --check for the new test, so run cargo fmt. Commit Messages fails with a 79 character header. This also duplicates #1163 and #1166, which are the same fix for #1041, so consolidate into one PR and close the other two.

@collinsezedike

Copy link
Copy Markdown
Collaborator

@Mamavee001 this stays as the single path for #1041, and #1163 and #1166 are being closed because they make the same fix. Four items remain before this can merge. Run cargo fmt, since Soroban Contract Tests fails on cargo fmt --check. Amend the commit header, currently 79 characters, to 72 or fewer. Carry the clean-address assertion from #1163, which checks that reading get_entry_time and get_principal for an address with no prior records creates no Entry or Principal keys. Carry the deposit assertion from #1166, which seeds legacy records, calls deposit, and checks that the fresh entry time and principal replace them, since this PR's doc says leftover records are cleaned up on the next deposit and the current test does not exercise that path.

@collinsezedike collinsezedike changed the title fix(vault): remove storage mutation from get_entry_time and get_principal views fix(vault): remove storage mutation from view getters Oct 25, 2026
@collinsezedike
collinsezedike force-pushed the fix/vault-views-storage-write branch from 24571ee to a4186a5 Compare October 25, 2026 08:36

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

The view getters no longer write storage when the position is zero. The new unit test seeds legacy Entry and Principal records and verifies both reads return zero while leaving them untouched, and cargo test -p meridian-vault passes with 100 tests. The architecture docs now describe the read-only behavior.

@collinsezedike
collinsezedike merged commit 93f7935 into drydocs:main Oct 25, 2026
10 of 11 checks passed
@collinsezedike

Copy link
Copy Markdown
Collaborator

@Mamavee001 Thank you for your contribution, and please star the repository if you found Meridian useful.

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.

[Bug] get_entry_time and get_principal write storage while reading

2 participants