From 8a6faba31ef761851ac9cc0c168454c8b7757a12 Mon Sep 17 00:00:00 2001 From: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com> Date: Thu, 5 Oct 2023 16:25:55 -0500 Subject: [PATCH] update .getCapability() to return nil if cap disallowed + tests (#160) Closes: #159 --- contracts/HybridCustody.cdc | 14 +++--- .../get_nft_provider_capability_optional.cdc | 20 +++++++++ test/CapabilityFactory_tests.cdc | 8 ---- test/HybridCustody_tests.cdc | 45 +++++++++++++------ test/test_helpers.cdc | 8 ++++ 5 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 scripts/test/get_nft_provider_capability_optional.cdc diff --git a/contracts/HybridCustody.cdc b/contracts/HybridCustody.cdc index 139de49..1c5795d 100644 --- a/contracts/HybridCustody.cdc +++ b/contracts/HybridCustody.cdc @@ -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. @@ -588,11 +588,15 @@ pub contract HybridCustody { } let acct = child.borrowAccount() - let cap = f!.getCapability(acct: acct, path: path) - - if path.getType() == Type() { - 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() && ( + self.filter.borrow()!.allowed(cap: cap) == false || + (self.getManagerCapabilityFilter()?.allowed(cap: cap) ?? true) == false + ) { + return nil } return cap diff --git a/scripts/test/get_nft_provider_capability_optional.cdc b/scripts/test/get_nft_provider_capability_optional.cdc new file mode 100644 index 0000000..85b8b06 --- /dev/null +++ b/scripts/test/get_nft_provider_capability_optional.cdc @@ -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())! as! MetadataViews.NFTCollectionData + + let nakedCap = childAcct.getCapability(path: collectionData.providerPath, type: Type<&{NonFungibleToken.Provider}>()) + + return returnsNil ? nakedCap == nil : nakedCap?.borrow<&{NonFungibleToken.Provider}>() != nil +} \ No newline at end of file diff --git a/test/CapabilityFactory_tests.cdc b/test/CapabilityFactory_tests.cdc index 2fba2c6..23708cf 100644 --- a/test/CapabilityFactory_tests.cdc +++ b/test/CapabilityFactory_tests.cdc @@ -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 diff --git a/test/HybridCustody_tests.cdc b/test/HybridCustody_tests.cdc index 6b767a5..0f3ce09 100644 --- a/test/HybridCustody_tests.cdc +++ b/test/HybridCustody_tests.cdc @@ -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() @@ -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]) @@ -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() { @@ -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) @@ -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() { @@ -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() { @@ -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() diff --git a/test/test_helpers.cdc b/test/test_helpers.cdc index 77203f7..c7365d4 100644 --- a/test/test_helpers.cdc +++ b/test/test_helpers.cdc @@ -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")