Skip to content

Fix medium and low findings from contract review#180

Merged
joshuahannan merged 1 commit into
mainfrom
fix/medium-low-findings
Apr 3, 2026
Merged

Fix medium and low findings from contract review#180
joshuahannan merged 1 commit into
mainfrom
fix/medium-low-findings

Conversation

@joshuahannan

@joshuahannan joshuahannan commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses three findings from the contract review. All 68 tests pass.

  • ChildAccount.setRedeemed previously used optional chaining (?.setRedeemed) which silently did nothing if OwnedAccount was not found at the expected storage path. This could leave OwnedAccount.parents stale — the parent would be tracked as active in the Manager but remain "pending" in the child forever. Now panics explicitly with a descriptive message.

  • publishToParent had a dead if branch checking whether the delegator storage slot was empty immediately after an assert that already guaranteed it was empty. Removed the redundant conditional — the delegator is now created unconditionally (as it always was in practice).

  • CapabilityDelegator.addCapability had no enforcement of the documented invariant that a capability type should exist in at most one partition (public or private). Added pre-conditions to reject additions that would create a cross-partition type collision.

🤖 Generated with Claude Code

- Fix #4: Explicitly panic in ChildAccount.setRedeemed when OwnedAccount
  is not found at expected storage path, preventing silent state
  inconsistency in the parents map
- Fix #5: Remove dead if-branch in publishToParent; the preceding assert
  already guarantees the storage slot is empty, making the condition
  always true
- Fix #7: Enforce cross-partition exclusivity in
  CapabilityDelegator.addCapability via pre-conditions, preventing the
  same capability type from existing in both public and private partitions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
contracts/HybridCustody.cdc 50.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@joshuahannan joshuahannan requested a review from jribbink March 25, 2026 16:13
@joshuahannan joshuahannan merged commit bd8eb37 into main Apr 3, 2026
1 check passed
@joshuahannan joshuahannan deleted the fix/medium-low-findings branch April 3, 2026 16:29
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.

3 participants