Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update .getCapability() to return nil if cap disallowed + tests #160

Merged
merged 2 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 m = acct.borrow<&HybridCustody.Manager>(from: HybridCustody.ManagerStoragePath)
?? panic("manager does not exist")

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

let d = ExampleNFT.resolveView(Type<MetadataViews.NFTCollectionData>())! as! MetadataViews.NFTCollectionData
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets try to use a more descriptive name than d here


let nakedCap = childAcct.getCapability(path: d.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