Skip to content

Fix #131: Storage optimisation + Fix #136: Cryptographic hardening for RefundVault - #251

Open
fredericklamar342-prog wants to merge 2 commits into
accensa:mainfrom
fredericklamar342-prog:fix/issues-131-136
Open

Fix #131: Storage optimisation + Fix #136: Cryptographic hardening for RefundVault#251
fredericklamar342-prog wants to merge 2 commits into
accensa:mainfrom
fredericklamar342-prog:fix/issues-131-136

Conversation

@fredericklamar342-prog

Copy link
Copy Markdown

Summary

This PR addresses both Issue #131 (storage layout optimisation) and Issue #136 (cryptographic malleability audit and hardening) for the RefundVault contract.


Issue #131 — Optimise Storage Layout and Instance Data Footprint

Problem: Yield-related keys (YieldStrategy, DeployedPrincipal, HarvestedYield, ReserveRatio, MaxDeployRatio) were stored in Instance storage, which is loaded on every contract invocation — including non-yield calls like deposit, refund, withdraw, pause, and admin transfers. This led to unnecessarily high read/write byte fees on the most common operations.

Fix:

  • Moved all five yield-related keys from Instance to Persistent storage.
  • Non-yield calls no longer load these keys, reducing per-invocation byte fees.
  • Persistent entries receive TTL bumping on every write via a new persist_yield_ttl helper.
  • get_yield_info() reads from Persistent storage.
  • Updated docs/storage-audit.md entries for the affected keys.

Acceptance criteria met:

  • ✅ Storage types are appropriately segregated based on data lifecycle.
  • ✅ TTL bumping logic is implemented for Persistent storage.
  • ✅ Non-yield operations no longer touch yield storage keys.

Issue #136 — Audit and Fix Cryptographic Malleability in RefundVault Signatures

Audit finding: RefundVault does not use custom signature verification — it relies entirely on Soroban's built-in require_auth() mechanism (Ed25519, inherently canonical). There are no ECDSA paths to malleate.

Hardening added (defense-in-depth):

  1. Domain Separator (get_domain_separator): A SHA-256 hash of the contract address, stored at initialize and never changed. Off-chain systems should bind signed authorizations to this value so a replay against a different vault deployment is rejected. Separate instances produce distinct separators.

  2. Monotonic Nonce (get_nonce): A u64 counter incremented on every successful state-changing call (deposit, refund, withdraw, deploy_to_yield, withdraw_from_yield, harvest_yield). Events carry the nonce so off-chain indexers can detect replays or reorderings.

Acceptance criteria met:

  • ✅ Signature canonicalization checks enforced (Soroban native auth — no custom paths).
  • ✅ Replay attack vectors mitigated with domain separators and strict nonces.
  • ✅ Tests simulate malleability/replay attacks and ensure they fail.

Tests added

Test Verifies
test_domain_separator_is_set_on_initialize Domain separator is a valid non-zero SHA-256 hash
test_domain_separator_differs_per_instance Two vault instances produce distinct separators
test_nonce_starts_at_zero Nonce initializes to 0
test_nonce_increments_on_deposit Deposit increments nonce
test_nonce_increments_on_refund Refund increments nonce
test_nonce_increments_on_withdraw Withdraw increments nonce
test_nonce_does_not_increment_on_failed_operation Failed calls do not bump nonce
test_nonce_is_strictly_monotonic Nonces increase monotonically across mixed operations

Event assertion tests (test_events_emitted, test_yield_deployed_event, test_yield_harvested_event) updated to verify the nonce field.


Files changed

  • contracts/refund-vault/src/lib.rs — Storage class changes, domain separator, nonce, event nonce fields
  • contracts/refund-vault/src/test.rs — New domain-separator/nonce tests, updated event assertions
  • contracts/refund-vault/src/yield_tests.rs — Updated event assertions for nonce field
  • CHANGELOG.md — Entries for both issues

Fixes #136
Fixes #131

…ic hardening (accensa#136)

Issue accensa#131 — Move yield-related keys (YieldStrategy, DeployedPrincipal,
HarvestedYield, ReserveRatio, MaxDeployRatio) from Instance to Persistent
storage so non-yield calls (deposit, refund, withdraw, pause) no longer
pay the read/write byte cost of loading them. Persistent entries receive
TTL bumping on every write via persist_yield_ttl.

Issue accensa#136 — Add domain separator (SHA-256 of the contract address,
stored at initialisation) and a monotonic operation nonce incremented on
every successful state-changing call. Events now carry the nonce so
off-chain indexers can detect replays or reorderings. Separate vault
instances produce distinct domain separators, preventing cross-contract
replay of signed authorisations. New getter functions get_domain_separator
and get_nonce expose these values. Tests verify nonce monotonicity,
cross-instance separator uniqueness, and nonce absence on failed calls.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@drips-wave

drips-wave Bot commented Aug 27, 2026

Copy link
Copy Markdown

@fredericklamar342-prog 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! 🚀

Learn more about application limits

@mergekeeper

mergekeeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

MergeKeeper review

Scope: in scope for linked issue #131.
Verdict: clean

The pull request successfully implements storage optimization for yield keys and cryptographic hardening with domain separators and monotonic nonces for RefundVault as requested in issues #131 and #136, complete with tests.

Reviewed commit: 16109cbd3101c0c23449cc5c6c73639cf5539684.
CI and merge eligibility are checked separately.

@mergekeeper

mergekeeper Bot commented Aug 28, 2026

Copy link
Copy Markdown

MergeKeeper merge status

Status: blocked
PR state: open
Mergeability: conflicts / not mergeable
Checked commit: 16109cbd3101c0c23449cc5c6c73639cf5539684.

Reason: One or more required CI checks failed.

Failing checks:

Next steps:

  1. Open the failing check details above and fix the reported error.
  2. Run the same checks locally where possible.
  3. Commit and push the fix.
  4. MergeKeeper will automatically re-review the updated PR.

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.

Audit and Fix Cryptographic Malleability in RefundVault Signatures Optimize Storage Layout and Instance Data Footprint in RefundVault

2 participants