Skip to content

Commit

Permalink
update .getCapability() to return nil if cap disallowed + tests (#160)
Browse files Browse the repository at this point in the history
Closes: #159
  • Loading branch information
sisyphusSmiling authored Oct 5, 2023
1 parent 84ab4b8 commit 8a6faba
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 26 deletions.
14 changes: 9 additions & 5 deletions contracts/HybridCustody.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ pub contract HybridCustody {

/// 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.
/// ensure that borrowing is permitted. If not allowed, nil is returned.
/// 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.
Expand All @@ -588,11 +588,15 @@ pub contract HybridCustody {
}

let acct = child.borrowAccount()

let cap = f!.getCapability(acct: acct, path: path)

if path.getType() == Type<PrivatePath>() {
assert(self.filter.borrow()!.allowed(cap: cap), message: "requested capability is not allowed")

// Check that private capabilities are allowed by either internal or manager filter (if assigned)
// If not allowed, return nil
if path.getType() == Type<PrivatePath>() && (
self.filter.borrow()!.allowed(cap: cap) == false ||
(self.getManagerCapabilityFilter()?.allowed(cap: cap) ?? true) == false
) {
return nil
}

return cap
Expand Down
20 changes: 20 additions & 0 deletions scripts/test/get_nft_provider_capability_optional.cdc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import "HybridCustody"

import "NonFungibleToken"
import "MetadataViews"
import "ExampleNFT"

// Verify that a child address borrowed as a child will let the parent borrow an NFT provider capability
pub fun main(parent: Address, child: Address, returnsNil: Bool): Bool {
let acct = getAuthAccount(parent)
let manager = acct.borrow<&HybridCustody.Manager>(from: HybridCustody.ManagerStoragePath)
?? panic("manager does not exist")

let childAcct = manager.borrowAccount(addr: child) ?? panic("child account not found")

let collectionData = ExampleNFT.resolveView(Type<MetadataViews.NFTCollectionData>())! as! MetadataViews.NFTCollectionData

let nakedCap = childAcct.getCapability(path: collectionData.providerPath, type: Type<&{NonFungibleToken.Provider}>())

return returnsNil ? nakedCap == nil : nakedCap?.borrow<&{NonFungibleToken.Provider}>() != nil
}
8 changes: 0 additions & 8 deletions test/CapabilityFactory_tests.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,4 @@ pub fun findExampleNFTCollectionType(_ owner: Test.Account) {
assert(borrowed, message: "failed to borrow delegator")
}

pub fun expectScriptFailure(_ scriptName: String, _ arguments: [AnyStruct]): String {
let scriptCode = loadCode(scriptName, "scripts")
let scriptResult = blockchain.executeScript(scriptCode, arguments)

assert(scriptResult.error != nil, message: "script error was expected but there is no error message")
return scriptResult.error!.message
}

// END SECTION - scripts used in tests
45 changes: 32 additions & 13 deletions test/HybridCustody_tests.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,33 @@ pub fun testChildAccount_getCapability() {
scriptExecutor("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
}

pub fun testChildAccount_getCapabilityReturnsNil() {
let dev = blockchain.createAccount()
let child = blockchain.createAccount()
let parent = blockchain.createAccount()

// Setup NFT Filter & Factory
txExecutor("dev-setup/setup_nft_filter_and_factory_manager.cdc", [dev], [accounts[exampleNFT]!.address, exampleNFT], nil, nil)
// Setup OwnedAccount in child account
txExecutor("hybrid-custody/setup_owned_account.cdc", [child], [nil, nil, nil], nil, nil)
// Publish ChildAccount for parent, factory & filter found in previously configured dev accoun
txExecutor("hybrid-custody/publish_to_parent.cdc", [child], [parent.address, dev.address, dev.address], nil, nil)
// Redeem ChildAccount as parent
txExecutor("hybrid-custody/redeem_account.cdc", [parent], [child.address, nil, nil], nil, nil)

// Configure NFT Collection in child account
setupNFTCollection(child)
// ExampleNFT Provider is allowed by Allowlist Filter, so should return valid Capability
let returnsValidCap = scriptExecutor("test/get_nft_provider_capability_optional.cdc", [parent.address, child.address, false])! as! Bool
Test.assertEqual(true, returnsValidCap)
// Remove types from filter
removeAllFilterTypes(dev, FilterKindAllowList)

// ExampleNFT Provider has been removed from Allowlist Filter - getCapability() should return nil
let returnsNil = scriptExecutor("test/get_nft_provider_capability_optional.cdc", [parent.address, child.address, true])! as! Bool
Test.assertEqual(true, returnsNil)
}

pub fun testChildAccount_getPublicCapability() {
let child = blockchain.createAccount()
let parent = blockchain.createAccount()
Expand Down Expand Up @@ -325,7 +352,7 @@ pub fun testGetCapability_ManagerFilterAllowed() {
setManagerFilterOnChild(child: child, parent: parent, filterAddress: filter.address)

let error = expectScriptFailure("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
assert(contains(error, "Capability is not allowed by this account's Parent"), message: "failed to find expected error message")
assert(contains(error, "capability not found"), message: "failed to find expected error message")

addTypeToFilter(filter, FilterKindAllowList, nftIdentifier)
scriptExecutor("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
Expand Down Expand Up @@ -354,7 +381,7 @@ pub fun testAllowlistFilterRemoveAllTypes() {
removeAllFilterTypes(filter, FilterKindAllowList)

let error = expectScriptFailure("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
assert(contains(error, "Capability is not allowed by this account's Parent"), message: "failed to find expected error message")
assert(contains(error, "capability not found"), message: "failed to find expected error message")
}

pub fun testDenyListFilterRemoveAllTypes() {
Expand All @@ -375,7 +402,7 @@ pub fun testDenyListFilterRemoveAllTypes() {
setManagerFilterOnChild(child: child, parent: parent, filterAddress: filter.address)

let error = expectScriptFailure("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
assert(contains(error, "Capability is not allowed by this account's Parent"), message: "failed to find expected error message")
assert(contains(error, "capability not found"), message: "failed to find expected error message")


removeAllFilterTypes(filter, FilterKindDenyList)
Expand All @@ -400,7 +427,7 @@ pub fun testGetCapability_ManagerFilterNotAllowed() {
setManagerFilterOnChild(child: child, parent: parent, filterAddress: filter.address)

let error = expectScriptFailure("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child.address])
assert(contains(error, "Capability is not allowed by this account's Parent"), message: "failed to find expected error message")
assert(contains(error, "capability not found"), message: "failed to find expected error message")
}

pub fun testGetPrivateCapabilityFromDelegator() {
Expand Down Expand Up @@ -813,7 +840,7 @@ pub fun testSetDefaultManagerFilter() {
addTypeToFilter(filter, FilterKindDenyList, nftIdentifier)

let error = expectScriptFailure("hybrid-custody/get_nft_provider_capability.cdc", [parent.address, child2.address])
assert(contains(error, "Capability is not allowed by this account's Parent"), message: "failed to find expected error message")
assert(contains(error, "capability not found"), message: "failed to find expected error message")
}

pub fun testPublishToParent_alreadyExists() {
Expand Down Expand Up @@ -1074,14 +1101,6 @@ pub fun getTestAccount(_ name: String): Test.Account {
return accounts[name]!
}

pub fun expectScriptFailure(_ scriptName: String, _ arguments: [AnyStruct]): String {
let scriptCode = loadCode(scriptName, "scripts")
let scriptResult = blockchain.executeScript(scriptCode, arguments)

assert(scriptResult.error != nil, message: "script error was expected but there is no error message")
return scriptResult.error!.message
}

pub fun setup() {
// actual test accounts
let parent = blockchain.createAccount()
Expand Down
8 changes: 8 additions & 0 deletions test/test_helpers.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ pub fun scriptExecutor(_ scriptName: String, _ arguments: [AnyStruct]): AnyStruc
return scriptResult.returnValue
}

pub fun expectScriptFailure(_ scriptName: String, _ arguments: [AnyStruct]): String {
let scriptCode = loadCode(scriptName, "scripts")
let scriptResult = blockchain.executeScript(scriptCode, arguments)

assert(scriptResult.error != nil, message: "script error was expected but there is no error message")
return scriptResult.error!.message
}

pub fun txExecutor(_ txName: String, _ signers: [Test.Account], _ arguments: [AnyStruct], _ expectedError: String?, _ expectedErrorType: ErrorType?): Bool {
let txCode = loadCode(txName, "transactions")

Expand Down

0 comments on commit 8a6faba

Please sign in to comment.