Nialexsan/non public pool by nialexsan · Pull Request #55 · onflow/FlowYieldVaults

nialexsan · 2025-09-10T14:15:36Z

No description provided.

…pool

remove logs

sisyphusSmiling

Biggest callout not captured here is that this setup only restricts access via our application. Without an offchain allowlist/blocklist, anyone with access to the URL can open a Tide. We'll need clarity on whether that's correct behavior or if the closed beta behavior should only enable known addresses. If the latter, we'll also need to consider where that list is stored and how that access is enforced.

sisyphusSmiling · 2025-09-18T19:48:44Z

+    // 1) Define an entitlement only the admin can issue
+    access(all) entitlement Admin
+
+    access(all) resource interface IBeta {}


We should remove this interface and interact only with the concrete type

sisyphusSmiling · 2025-09-18T19:49:03Z

+    access(all) entitlement Admin
+
+    access(all) resource interface IBeta {}
+    access(all) resource BetaBadge: IBeta {}


We should add the address of the user this was issued for

sisyphusSmiling · 2025-09-18T19:51:59Z

+
+    // 2) A small in-account helper resource that performs privileged ops
+    access(all) resource AdminHandle {
+        access(Admin) fun grantBeta(addr: Address): Capability<&{TidalYieldClosedBeta.IBeta}> {


This should return an authorized capability on the concrete type

sisyphusSmiling · 2025-09-18T19:52:40Z

+        let ctrl = self.account.capabilities.storage.getController(byCapabilityID: id)
+            ?? panic("Missing controller for recorded cap ID")
+        ctrl.delete()
+        self.issuedCapIDs.remove(key: addr)


We'll also want to load & destroy the BetaBadge in storage as well.

I'm also noticing that there is not retained history of an address having been revoked. Without an allowlist, this could result in a previously revoked address gaining access again. If we want to capture that information, we may want to either keep a revoked: {Address: Bool} mapping or update issuedCapIDs to include this information. Maybe something like {Address: AccessInfo} where AccessInfo is

access(all) struct AccessInfo { access(all) let capID: UInt64 access(all) let isRevoked: Bool }

sisyphusSmiling · 2025-09-18T19:55:31Z

        }
        /// Creates a new Tide executing the specified Strategy with the provided funds
-        access(all) fun createTide(strategyType: Type, withVault: @{FungibleToken.Vault}) {
+        access(all) fun createTide(betaRef: &{TidalYieldClosedBeta.IBeta}, strategyType: Type, withVault: @{FungibleToken.Vault}) {


This will need to be updated to use an authorized reference on the concrete type. We'll also want to accept the same reference type on deposit to ensure only valid beta participants can add funds to their Tide.

We may also want to require a reference for addTide, withdrawTide and createManager.

Also, since we're adding address to the beta resource, we should check that against self.owner!.address so restrict third-party access via copied Capabilities.

sisyphusSmiling · 2025-09-18T20:05:23Z

+    let capExists = getAccount(addr).capabilities.exists(TidalYieldClosedBeta.BetaBadgePublicPath)
+    if !capExists {
+        return false
+    }
+    let betaCapID = TidalYieldClosedBeta.getBetaCapID(addr)
+    let existingCap = getAccount(addr).capabilities.get<&{TidalYieldClosedBeta.IBeta}>(TidalYieldClosedBeta.BetaBadgePublicPath)
+
+    return existingCap.id != nil && betaCapID != nil


Suggested change

let capExists = getAccount(addr).capabilities.exists(TidalYieldClosedBeta.BetaBadgePublicPath)

if !capExists {

return false

}

let betaCapID = TidalYieldClosedBeta.getBetaCapID(addr)

let existingCap = getAccount(addr).capabilities.get<&{TidalYieldClosedBeta.IBeta}>(TidalYieldClosedBeta.BetaBadgePublicPath)

return existingCap.id != nil && betaCapID != nil

let acct = getAuthAccount<auth(Storage) &Account>(addr)

let betaCapID = TidalYieldClosedBeta.getBetaCapID(addr)

let existingCap = acct.storage.borrow<Capability<auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge>>(TidalYieldClosedBeta.UserBetaCapStoragePath)

return betaCapID != nil && existingCap?.id == betaCapID

sisyphusSmiling · 2025-09-18T20:06:50Z

+        let copyCap = user.capabilities.storage.issue<&{TidalYieldClosedBeta.IBeta}>(p)
+
+        let pubPath = TidalYieldClosedBeta.BetaBadgePublicPath
+
+        // If anything is already published there, remove it first
+        let existingAny = user.capabilities.exists(pubPath)
+        if existingAny {
+            user.capabilities.unpublish(pubPath)
+        }
+
+        user.capabilities.publish(
+            copyCap, 
+            at: TidalYieldClosedBeta.BetaBadgePublicPath
+        )


We shouldn't need this after the rest of the changes

sisyphusSmiling · 2025-09-18T20:10:40Z

+    prepare(
+        admin: auth(Capabilities) &Account,
+    ) {
+        let adminCap: Capability<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle> =
+            admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(
+                TidalYieldClosedBeta.AdminHandleStoragePath
+            )
+        let handle: auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle =
+            adminCap.borrow() ?? panic("Missing AdminHandle")
+
+        handle.revokeByAddress(addr: addr)


No need to issue a capability

Suggested change

prepare(

admin: auth(Capabilities) &Account,

) {

let adminCap: Capability<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle> =

admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(

TidalYieldClosedBeta.AdminHandleStoragePath

)

let handle: auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle =

adminCap.borrow() ?? panic("Missing AdminHandle")

handle.revokeByAddress(addr: addr)

prepare(admin: auth(BorrowValue) &Account) {

let handle = admin.storage.borrow<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(

from: TidalYieldClosedBeta.AdminHandleStoragePath

)?.revokeByAddress(addr: addr)

?? panic("Missing AdminHandle")

sisyphusSmiling · 2025-09-18T20:10:58Z

    let manager: &TidalYield.TideManager
    let strategy: Type
    let depositVault: @{FungibleToken.Vault}
+    let betaRef: &{TidalYieldClosedBeta.IBeta}


Update to use concrete type

sisyphusSmiling · 2025-09-19T18:05:07Z

+    access(all) resource BetaBadge {
+        access(all) let _owner: Address
+        init(_ o: Address) {
+            self._owner = o
+        }
+        access(all) view fun getOwner(): Address {
+            return self._owner
+        }
+    }


np: preference for assignedOwner or assignedTo to avoid confusion with self.owner

sisyphusSmiling · 2025-09-19T18:09:54Z

        let adminCap: Capability<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle> =
-        admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(
-            TidalYieldClosedBeta.AdminHandleStoragePath
-        )
+            admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(
+                TidalYieldClosedBeta.AdminHandleStoragePath
+            )
        let handle: auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle =
-        adminCap.borrow() ?? panic("Missing AdminHandle")
+            adminCap.borrow() ?? panic("Missing AdminHandle")


Suggested change

let adminCap: Capability<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle> =

admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(

TidalYieldClosedBeta.AdminHandleStoragePath

)

admin.capabilities.storage.issue<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(

TidalYieldClosedBeta.AdminHandleStoragePath

)

let handle: auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle =

adminCap.borrow() ?? panic("Missing AdminHandle")

adminCap.borrow() ?? panic("Missing AdminHandle")

let handle = admin.capabilities.storage.borrow<auth(TidalYieldClosedBeta.Admin) &TidalYieldClosedBeta.AdminHandle>(

from: TidalYieldClosedBeta.AdminHandleStoragePath

) ?? panic("Missing AdminHandle")

sisyphusSmiling · 2025-09-19T18:13:41Z

+        access(all) fun createTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, strategyType: Type, withVault: @{FungibleToken.Vault}) {
            let balance = withVault.balance


Suggested change

access(all) fun createTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, strategyType: Type, withVault: @{FungibleToken.Vault}) {

let balance = withVault.balance

access(all) fun createTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, strategyType: Type, withVault: @{FungibleToken.Vault}) {

pre {

betaRef.getOwner() == self.owner?.address:

"BetaBadge may only be used by its assigned owner \(betaRef.getOwner()) to create a Tide"

TidalYieldClosedBeta.issuedCapIDs[betaRef.getOwner()]?.isRevoked ?? true == false:

"Beta access for \(betaRef.getOwner()) has been revoked - may only withdraw from existing Tides"

}

let balance = withVault.balance

sisyphusSmiling · 2025-09-19T18:17:33Z

+        access(all) fun depositToTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ id: UInt64, from: @{FungibleToken.Vault}) {
            pre {
                self.tides[id] != nil:
                "No Tide with ID \(id) found"
            }


Suggested change

access(all) fun depositToTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ id: UInt64, from: @{FungibleToken.Vault}) {

pre {

self.tides[id] != nil:

"No Tide with ID \(id) found"

}

access(all) fun depositToTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ id: UInt64, from: @{FungibleToken.Vault}) {

pre {

self.tides[id] != nil:

"No Tide with ID \(id) found"

betaRef.getOwner() == self.owner?.address:

"BetaBadge may only be used by its assigned owner \(betaRef.getOwner()) to deposit to a Tide"

TidalYieldClosedBeta.issuedCapIDs[betaRef.getOwner()]?.isRevoked ?? true == false:

"Beta access for \(betaRef.getOwner()) has been revoked - may only withdraw from existing Tides"

}

sisyphusSmiling · 2025-09-19T18:18:36Z

+        access(all) fun addTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ tide: @Tide) {
            pre {
                self.tides[tide.uniqueID.id] == nil:
                "Collision with Tide ID \(tide.uniqueID.id) - a Tide with this ID already exists"


Suggested change

access(all) fun addTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ tide: @Tide) {

pre {

self.tides[tide.uniqueID.id] == nil:

"Collision with Tide ID \(tide.uniqueID.id) - a Tide with this ID already exists"

access(all) fun addTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, _ tide: @Tide) {

pre {

self.tides[tide.uniqueID.id] == nil:

"Collision with Tide ID \(tide.uniqueID.id) - a Tide with this ID already exists"

TidalYieldClosedBeta.issuedCapIDs[betaRef.getOwner()]?.isRevoked ?? true == false:

"Beta access for \(betaRef.getOwner()) has been revoked - may only withdraw from existing Tides"

}

sisyphusSmiling · 2025-09-19T18:20:39Z

+    access(all) fun createTideManager(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge): @TideManager {
        return <-create TideManager()
    }


Suggested change

access(all) fun createTideManager(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge): @TideManager {

return <-create TideManager()

}

access(all) fun createTideManager(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge): @TideManager {

pre {

TidalYieldClosedBeta.issuedCapIDs[betaRef.getOwner()]?.isRevoked ?? true == false:

"Beta access for \(betaRef.getOwner()) has been revoked - may only withdraw from existing Tides"

}

return <-create TideManager()

}

sisyphusSmiling · 2025-09-19T18:36:27Z

+            pre {
+                self.tides[id] != nil:
+                "No Tide with ID \(id) found"
+            }


From our offline convo yesterday, I think we still want users to be able to withdraw after being revoked, but we should include the same check as other method if not.

Suggested change

pre {

self.tides[id] != nil:

"No Tide with ID \(id) found"

}

pre {

self.tides[id] != nil:

"No Tide with ID \(id) found"

TidalYieldClosedBeta.issuedCapIDs[betaRef.getOwner()]?.isRevoked ?? true == false:

"Beta access for \(betaRef.getOwner()) has been revoked - may only withdraw from existing Tides"

}

sisyphusSmiling · 2025-09-19T20:11:47Z

+        if (addr == nil) {
+            return false
+        }
+        // Must have a live, non-revoked record for this address
+        let recordedID: UInt64? = self.getBetaCapID(addr!); 
+        if recordedID == nil {
+            return false
+        }
+
+        if self.issuedCapIDs[addr!]?.isRevoked == true {
+            return false
+        }
+
+        if betaRef.getOwner() != addr {
+            return false
+        }
+
+        return true 
+    }


I think making assertions with relevant messages would be more informative about the reason for failure, but functionally this seems to cover all the bases. I don't think it's a blocker since this is only temporarily necessary.

nialexsan added 29 commits September 2, 2025 16:20

update refs

c3a0c9c

fix paths

0ac93c5

typo

f25a49c

fix paths

98fac2e

fix path

6fc5cda

add beta access to tests

74b7b88

typo

b68db17

align changes with closed beta contract

950d8aa

beta badge gate

77f7771

e2e test

910f7c2

fix path

5651bb6

fix e2e

c25b505

tweak script

ae2643d

Merge remote-tracking branch 'origin/main' into nialexsan/non-public-…

b0b1fed

…pool

Merge remote-tracking branch 'origin/main' into nialexsan/non-public-…

4d78167

…pool

fix tests

915aca2

recursive e2e modules

d4d2118

deps

a0d5339

update flow.json

bedc6e6

move grant beta to setup script

4e8fefc

fix path

7b1bf2c

update ref

724f335

token tracking

1759357

tweak cap

47eb7ea

tweak transaction

debb161

fix get beta cap check

18d91bb

tweak beta caps

0f8e5d2

rename store path

051a8ab

remove unused path

a896092

nialexsan requested a review from sisyphusSmiling September 18, 2025 16:40

nialexsan marked this pull request as ready for review September 18, 2025 16:40

nialexsan commented Sep 18, 2025

View reviewed changes

Comment thread cadence/transactions/tidal-yield/admin/grant_beta.cdc Outdated

Comment thread cadence/transactions/tidal-yield/admin/grant_beta.cdc Outdated

Comment thread cadence/transactions/tidal-yield/admin/grant_beta.cdc Outdated

Apply suggestions from code review

5644cba

remove logs

sisyphusSmiling reviewed Sep 18, 2025

View reviewed changes

nialexsan added 8 commits September 18, 2025 19:50

address PR comments

4c40797

tweak script

4702df8

fix beta ref

3ddd107

fix refs

8ebb062

address comments

f969d70

fix missing import

fdbb659

fix access

bd2f720

fix ref

a2e9fa6

sisyphusSmiling self-requested a review September 19, 2025 18:03

sisyphusSmiling reviewed Sep 19, 2025

View reviewed changes

nialexsan added 2 commits September 19, 2025 16:03

address comments

02ac093

update ref

ec2e960

sisyphusSmiling approved these changes Sep 19, 2025

View reviewed changes

nialexsan added 2 commits September 19, 2025 16:22

fix access

cec275b

assertions

d4b9bc4

nialexsan merged commit 16af405 into main Sep 19, 2025
4 checks passed

nialexsan deleted the nialexsan/non-public-pool branch September 19, 2025 20:59

		access(all) fun createTide(betaRef: auth(TidalYieldClosedBeta.Beta) &TidalYieldClosedBeta.BetaBadge, strategyType: Type, withVault: @{FungibleToken.Vault}) {
		let balance = withVault.balance

Uh oh!

Conversation

nialexsan commented Sep 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sisyphusSmiling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants