Skip to content

Commit bd8eb37

Browse files
joshuahannanclaude
andauthored
Fix medium and low findings from contract review (#180)
## 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](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ce985d1 commit bd8eb37

2 files changed

Lines changed: 11 additions & 5 deletions

File tree

contracts/CapabilityDelegator.cdc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ access(all) contract CapabilityDelegator {
128128
access(Add) fun addCapability(cap: Capability, isPublic: Bool) {
129129
pre {
130130
cap.check<&AnyResource>(): "Invalid Capability provided"
131+
// Enforce the invariant that a capability type exists in at most one partition.
132+
// Public and private capabilities of the same type cannot and should not be mixed.
133+
!isPublic || self.privateCapabilities[cap.getType()] == nil:
134+
"Capability type already exists in the private partition"
135+
isPublic || self.publicCapabilities[cap.getType()] == nil:
136+
"Capability type already exists in the public partition"
131137
}
132138
if isPublic {
133139
self.publicCapabilities.insert(key: cap.getType(), cap)

contracts/HybridCustody.cdc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,9 @@ access(all) contract HybridCustody {
666666
///
667667
access(contract) fun setRedeemed(_ addr: Address) {
668668
let acct = self.childCap.borrow()!._borrowAccount()
669-
acct.storage.borrow<&OwnedAccount>(from: HybridCustody.OwnedAccountStoragePath)?.setRedeemed(addr)
669+
let ownedAcct = acct.storage.borrow<&OwnedAccount>(from: HybridCustody.OwnedAccountStoragePath)
670+
?? panic("OwnedAccount not found at expected storage path during setRedeemed callback")
671+
ownedAcct.setRedeemed(addr)
670672
}
671673

672674
/// Returns a reference to the stored delegator, generally used for arbitrary Capability retrieval
@@ -868,10 +870,8 @@ access(all) contract HybridCustody {
868870
assert(acct.storage.borrow<&AnyResource>(from: capDelegatorStorage) == nil, message: "conflicting resource found in capability delegator storage slot for parentAddress")
869871
assert(acct.storage.borrow<&AnyResource>(from: childAccountStorage) == nil, message: "conflicting resource found in child account storage slot for parentAddress")
870872

871-
if acct.storage.borrow<&CapabilityDelegator.Delegator>(from: capDelegatorStorage) == nil {
872-
let delegator <- CapabilityDelegator.createDelegator()
873-
acct.storage.save(<-delegator, to: capDelegatorStorage)
874-
}
873+
let newDelegator <- CapabilityDelegator.createDelegator()
874+
acct.storage.save(<-newDelegator, to: capDelegatorStorage)
875875

876876
let capDelegatorPublic = PublicPath(identifier: capDelegatorIdentifier)!
877877

0 commit comments

Comments
 (0)