Skip to content

Commit

Permalink
Consolidate OwnedAccount setup txn & comment updates (#142)
Browse files Browse the repository at this point in the history
Closes: #136 

This PR adds comments to HybridCustody methods, with focus on helping
readers distinguish Capability retrieval methods, a concern raised by
@btspoony.

Also consolidates `OwnedAccount` setup transactions, making `Display`
fields optional transaction parameters as raised by @lmcmz
  • Loading branch information
sisyphusSmiling authored Aug 8, 2023
1 parent ecd55d4 commit 0e3b350
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 120 deletions.
40 changes: 29 additions & 11 deletions contracts/HybridCustody.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,14 @@ pub contract HybridCustody {
// Don't emit an event if nothing was removed
}

/// Removes the owned Capabilty on the specified account, relinquishing access to the account and publishes a
/// Capability for the specified account. See `OwnedAccount.giveOwnership()` for more details on this method.
///
/// **NOTE:** The existence of this method does not imply that it is the only way to receive access to a
/// OwnedAccount Capability or that only the labeled `to` account has said access. Rather, this is a convenient
/// mechanism intended to easily transfer 'root' access on this account to another account and an attempt to
/// minimize access vectors.
///
pub fun giveOwnership(addr: Address, to: Address) {
let acct = self.ownedAccounts.remove(key: addr)
?? panic("account not found")
Expand Down Expand Up @@ -562,9 +570,13 @@ pub contract HybridCustody {
self.filter = cap
}

// The main function to a child account's capabilities from a parent account. When a PrivatePath type is used,
// the CapabilityFilter will be borrowed and the Capability being returned will be checked against it to ensure
// that borrowing is permitted
/// The main function to a child account's capabilities from a parent account. When a PrivatePath type is used,
/// the CapabilityFilter will be borrowed and the Capability being returned will be checked against it to
/// ensure that borrowing is permitted.
/// Also know that this method retrieves Capabilities via the CapabilityFactory path. To retrieve arbitrary
/// Capabilities, see `getPrivateCapFromDelegator()` and `getPublicCapFromDelegator()` which use the
/// `Delegator` retrieval path.
///
pub fun getCapability(path: CapabilityPath, type: Type): Capability? {
let child = self.childCap.borrow() ?? panic("failed to borrow child account")

Expand All @@ -584,26 +596,29 @@ pub contract HybridCustody {
return cap
}

/// Retrieves a private Capability from the Delegator or nil none is found of the given type
/// Retrieves a private Capability from the Delegator or nil none is found of the given type. Useful for
/// arbitrary Capability retrieval
///
pub fun getPrivateCapFromDelegator(type: Type): Capability? {
if let p = self.delegator.borrow() {
return p.getPrivateCapability(type)
if let d = self.delegator.borrow() {
return d.getPrivateCapability(type)
}

return nil
}

/// Retrieves a public Capability from the Delegator or nil none is found of the given type
/// Retrieves a public Capability from the Delegator or nil none is found of the given type. Useful for
/// arbitrary Capability retrieval
///
pub fun getPublicCapFromDelegator(type: Type): Capability? {
if let p = self.delegator.borrow() {
return p.getPublicCapability(type)
if let d = self.delegator.borrow() {
return d.getPublicCapability(type)
}
return nil
}

/// Enables retrieval of public Capabilities of the given type from the specified path or nil if none is found
/// Enables retrieval of public Capabilities of the given type from the specified path or nil if none is found.
/// Callers should be aware this method uses the `CapabilityFactory` retrieval path.
///
pub fun getPublicCapability(path: PublicPath, type: Type): Capability? {
return self.getCapability(path: path, type: type)
Expand All @@ -624,7 +639,7 @@ pub contract HybridCustody {
}
}

/// Returns a reference to the stored delegator
/// Returns a reference to the stored delegator, generally used for arbitrary Capability retrieval
///
pub fun borrowCapabilityDelegator(): &CapabilityDelegator.Delegator? {
let path = HybridCustody.getCapabilityDelegatorIdentifier(self.parent)
Expand Down Expand Up @@ -660,6 +675,7 @@ pub contract HybridCustody {

/// Callback to enable parent-initiated removal all the child account and its associated resources &
/// Capabilities
///
access(contract) fun parentRemoveChildCallback(parent: Address) {
if !self.childCap.check() {
return
Expand Down Expand Up @@ -970,6 +986,8 @@ pub contract HybridCustody {
emit OwnershipGranted(ownedAcctID: self.uuid, child: self.acct.address, previousOwner: self.getOwner(), pendingOwner: to)
}

/// Revokes all keys on the underlying account
///
pub fun revokeAllKeys() {
let acct = self.borrowAccount()

Expand Down
8 changes: 4 additions & 4 deletions test/HybridCustody_tests.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub fun testSetupOwnedAccountAndPublishRedeemed() {

let parent = blockchain.createAccount()

txExecutor("hybrid-custody/setup_owned_account_and_publish_to_parent.cdc", [child], [parent.address, factory.address, filter.address], nil, nil)
txExecutor("hybrid-custody/setup_owned_account_and_publish_to_parent.cdc", [child], [parent.address, factory.address, filter.address, nil, nil, nil], nil, nil)

txExecutor("hybrid-custody/redeem_account.cdc", [parent], [child.address, nil, nil], nil, nil)

Expand All @@ -118,7 +118,7 @@ pub fun testSetupOwnedAccountWithDisplayPublishRedeemed() {


txExecutor(
"hybrid-custody/setup_owned_account_with_display_and_publish_to_parent.cdc",
"hybrid-custody/setup_owned_account_and_publish_to_parent.cdc",
[child],
[parent.address, factory.address, filter.address, name, desc, thumbnail],
nil,
Expand Down Expand Up @@ -624,7 +624,7 @@ pub fun testSetupOwnedAccountWithDisplay() {
let desc = "description"
let thumbnail = "https://example.com/test.jpeg"

txExecutor("hybrid-custody/setup_owned_account_with_display.cdc", [acct], [name, desc, thumbnail], nil, nil)
txExecutor("hybrid-custody/setup_owned_account.cdc", [acct], [name, desc, thumbnail], nil, nil)
assert(scriptExecutor("hybrid-custody/metadata/assert_owned_account_display.cdc", [acct.address, name, desc, thumbnail])! as! Bool, message: "failed to match display")
}

Expand Down Expand Up @@ -838,7 +838,7 @@ pub fun setupOwnedAccount(_ acct: Test.Account, _ filterKind: String) {
setupFT(acct)


txExecutor("hybrid-custody/setup_owned_account.cdc", [acct], [], nil, nil)
txExecutor("hybrid-custody/setup_owned_account.cdc", [acct], [nil, nil, nil], nil, nil)
}

pub fun setupFactoryManager(_ acct: Test.Account) {
Expand Down
24 changes: 17 additions & 7 deletions transactions/hybrid-custody/setup_owned_account.cdc
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#allowAccountLinking

import "HybridCustody"

import "CapabilityFactory"
import "CapabilityDelegator"
import "CapabilityFilter"

import "MetadataViews"

transaction {
import "HybridCustody"

/// This transaction configures an OwnedAccount in the signer if needed and configures its Capabilities per
/// HybridCustody's intended design. If Display values are specified (as recommended), they will be set on the
/// signer's OwnedAccount.
///
transaction(name: String?, desc: String?, thumbnailURL: String?) {
prepare(acct: AuthAccount) {
var acctCap = acct.getCapability<&AuthAccount>(HybridCustody.LinkedAccountPrivatePath)
if !acctCap.check() {
Expand All @@ -20,6 +20,16 @@ transaction {
acct.save(<-ownedAccount, to: HybridCustody.OwnedAccountStoragePath)
}

let owned = acct.borrow<&HybridCustody.OwnedAccount>(from: HybridCustody.OwnedAccountStoragePath)
?? panic("owned account not found")

// Set the display metadata for the OwnedAccount
if name != nil && desc != nil && thumbnailURL != nil {
let thumbnail = MetadataViews.HTTPFile(url: thumbnailURL!)
let display = MetadataViews.Display(name: name!, description: desc!, thumbnail: thumbnail!)
owned.setDisplay(display)
}

// check that paths are all configured properly
acct.unlink(HybridCustody.OwnedAccountPrivatePath)
acct.link<&HybridCustody.OwnedAccount{HybridCustody.BorrowableAccount, HybridCustody.OwnedAccountPublic, MetadataViews.Resolver}>(HybridCustody.OwnedAccountPrivatePath, target: HybridCustody.OwnedAccountStoragePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ import "CapabilityDelegator"
/// using CapabilityFactory.Manager and CapabilityFilter.Filter Capabilities from the given addresses. A
/// Capability on the ChildAccount is then published to the specified parent account.
///
transaction(parent: Address, factoryAddress: Address, filterAddress: Address) {
transaction(
parent: Address,
factoryAddress: Address,
filterAddress: Address,
name: String?,
desc: String?,
thumbnailURL: String?
) {

prepare(acct: AuthAccount) {
// Configure OwnedAccount if it doesn't exist
Expand All @@ -33,6 +40,13 @@ transaction(parent: Address, factoryAddress: Address, filterAddress: Address) {

let owned = acct.borrow<&HybridCustody.OwnedAccount>(from: HybridCustody.OwnedAccountStoragePath)
?? panic("owned account not found")

// Set the display metadata for the OwnedAccount
if name != nil && desc != nil && thumbnailURL != nil {
let thumbnail = MetadataViews.HTTPFile(url: thumbnailURL!)
let display = MetadataViews.Display(name: name!, description: desc!, thumbnail: thumbnail!)
owned.setDisplay(display)
}

// Get CapabilityFactory & CapabilityFilter Capabilities
let factory = getAccount(factoryAddress).getCapability<&CapabilityFactory.Manager{CapabilityFactory.Getter}>(CapabilityFactory.PublicPath)
Expand Down
37 changes: 0 additions & 37 deletions transactions/hybrid-custody/setup_owned_account_with_display.cdc

This file was deleted.

This file was deleted.

0 comments on commit 0e3b350

Please sign in to comment.