Skip to content

Fix style and low-severity findings from contract review#179

Merged
joshuahannan merged 2 commits intomainfrom
fix/style-and-low-findings
Mar 24, 2026
Merged

Fix style and low-severity findings from contract review#179
joshuahannan merged 2 commits intomainfrom
fix/style-and-low-findings

Conversation

@joshuahannan
Copy link
Copy Markdown
Contributor

@joshuahannan joshuahannan commented Mar 24, 2026

Summary

Addresses style and low-severity findings identified during a comprehensive Cadence contract review. All 68 tests pass.

  • Replace nil-check + force-unwrap anti-pattern with if let in Manager.borrowAccount, Manager.borrowAccountPublic, ChildAccount.getControllerIDForType, OwnedAccount.getControllerIDForType
  • Collapse redundant double borrow of filter capability in publishToParent event emission
  • Add missing view modifier to getAllPrivate in CapabilityDelegator interface and implementation (consistent with getAllPublic)
  • Fix copy-paste errors in AllowlistFilter doc comments — "denied types" corrected to "allowed types"
  • Remove duplicate comment line on ManagerPublic interface
  • Fix typo "itelf" → "itself"
  • Remove spurious leftover "AccountCapabilityController" fragment from CapabilityDelegator doc comment
  • Remove unresolved inline TODO from seal(); design concern tracked in issue seal() always revokes all keys — need a giveOwnership path that preserves keys #178
  • Decompose complex one-liner filter check in ChildAccount.getCapability into named variables with explanatory comments
  • Add CLAUDE.md and AGENTS.md (symlink) for Claude Code guidance
  • Add REPORT.md to .gitignore

Related

🤖 Generated with Claude Code

joshuahannan and others added 2 commits March 24, 2026 11:56
- Fix #7: Replace nil-check + force-unwrap with if-let in
  Manager.borrowAccount, Manager.borrowAccountPublic,
  ChildAccount.getControllerIDForType, OwnedAccount.getControllerIDForType
- Fix #8: Collapse double borrow of filter capability in publishToParent event
- Fix #10: Add missing `view` to getAllPrivate in CapabilityDelegator
  interface and implementation
- Fix #12: Correct copy-paste errors in AllowlistFilter doc comments
  ("denied types" -> "allowed types")
- Fix #13: Remove duplicate comment line on ManagerPublic interface
- Fix #14: Fix typo "itelf" -> "itself"
- Fix #15: Remove spurious "AccountCapabilityController" fragment from
  CapabilityDelegator doc comment
- Fix #16: Remove unresolved TODO from seal(); tracked in issue #178
- Fix #17: Decompose complex filter check in ChildAccount.getCapability
  into named variables with explanatory comments
- Add CLAUDE.md and AGENTS.md (symlink) for Claude Code guidance
- Add REPORT.md to .gitignore

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

Codecov Report

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

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

📢 Thoughts on this report? Let us know!

@joshuahannan joshuahannan requested a review from Kay-Zee March 24, 2026 18:32
@joshuahannan joshuahannan merged commit ce985d1 into main Mar 24, 2026
1 check passed
@joshuahannan joshuahannan deleted the fix/style-and-low-findings branch March 24, 2026 21:31
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.

4 participants