Fee Contract Improvements by glitch003 · Pull Request #367 · LIT-Protocol/Vincent

glitch003 · 2025-10-14T17:29:59Z

Description

This PR contains a few changes:

Makes it so that the user actually holds the Morpho and Aave receipt tokens, and the user approves the fee contract to withdraw on their behalf. This means that bonus rewards and airdrops will go to the user, not the fee contract, and the user can claim them directly without any involvement from the fee contract. This also means the user can easily bypass the fee by using WalletConnect, but this tradeoff is acceptable, to let them claim bonus rewards and airdrops.
Index everything by appId, so that we know which apps are earning fees
Add an app developer fee split into the fee contracts. All fees are split according to a percentage (currently 10%) where the Lit Foundation gets some (10%) and the app developer gets the rest (90%)
The app developer can withdraw their fees by using the app manager wallet
This fee collection system is unified between the app developer and the lit foundation: the Lit foundation has App Id 0 in the mapping that maps app id to collected fees. The Lit foundation withdraws their fees the same way as an app dev, except they must use the diamond owner account, and pass app id 0.
Added events for most things
Eliminated the userVaultOrPoolAssetAddresses function and storage, which was used to help the user track their deposits. Since the user actually holds their own deposits now, this is unnecessary, since they can just use walletconnect to see their balances on morpho or aave.

Type of change

Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Forge fork tests for all contracts

…, since we already took out the fee

…ect rewards

vercel · 2025-10-14T17:30:05Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
vincent-app-dashboard	Ready	Preview	Comment	Nov 5, 2025 1:29am

…sets now and so they can just look onchain or use walletconnect

Copilot

Pull Request Overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

packages/libs/contracts-sdk/contracts/fees/facets/AavePerfFeeFacet.sol:1

Corrected spelling of 'recieve' to 'receive' in comment.

// SPDX-License-Identifier: MIT

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

DashKash54

For the App to use our FeeContract they have to get the AgentPKP to approve our FeeContract and this approval exists indefinitely until revoked? This might not be an issue since the FeeContract ensure that the AgentPKP (msg.sender) always receives either the aToken/vaultShares or the withdraw amount- so funds can't be send to anyone else or get stuck?

DashKash54 · 2025-10-17T23:21:14Z

We don't have an upgrade script for FeeContracts?

we just wrote it and haven't deployed the initial version yet so no need for an upgrade script yet

DashKash54 · 2025-10-19T18:22:06Z

Doesn't naming all the PerfFeeFacet provider as VAULT_PROVIDER cause an issue while diamond updating the logic of an existing facet since the storage is shared in the entry Diamond contract? We should change its name to AAVE_VAULT_PROVIDER?

nope, it's a private variable for each facet, so it's not shared between them at all

DashKash54 · 2025-10-19T18:30:10Z

-        // remove the pool asset address from the set of vault or pool asset addresses
-        // so the user can't find their deposits later
-        LibFeeStorage.getStorage().userVaultOrPoolAssetAddresses[msg.sender].remove(poolAsset);
+        delete LibFeeStorage.getStorage().deposits[appId][msg.sender][poolAsset];


What if user provides amount less than the deposit amount then we will delete their entry from Deposit. So after withdraw they will have the remaining amount still in the pool which they can't withdraw from the FeeContract as we would have deleted their entry.

Since we're just tracking the user's deposit per app per pool/vault supporting partial withdrawal seems imperative otherwise we'll end up in a situation where the app is expected to enforce the total withdraw amount? Or we should enforce that the withdraw amount equals the deposit amount

What are the complexities for allowing partial withdraw we'd just deduct the withdraw amount and add an Reentrancy guard in the FeeContract?

yeah good point. we could remove the amount parameter and have the contract determine it, by looking at the user's balance of aTokens. this would enforce on-chain that they must withdraw everything they have.

allowing partial withdraws gets kinda confusing IMO. like, say I put in $100. and i waited 1 year at 10% apy so it's now $110. now I want to withdraw $50. they have $10 profit total, and we need to take a 10% fee of this profit (so, $1). so how much do we take out now? i suppose it's $50 / $110 = 0.454545454 which is the share of their total assets they're removing. so we calculate our total fee on the entire deposit ($1) and then apply that share, so take out $0.45 as fee.

and then we need to like, adjust the original deposited amount to account for this, so when they come back and withdraw the other $60 that's in the vault, we calculate profit correctly on that. so we adjust their original deposit amount to $50. and then they come back, and want to withdraw everything, so we calculate the profit ($10) and take our fee out of that ($1). but then we've collected $1.45 in fees total when we should have collected $1 in fees total?

i think it's possible to do this, it's just the math gets complex and confusing very fast and it seems like the feature of partial withdraws isn't worth the extra math

…ty - need to fix tests and mocks

…mpile but tests don't

Copilot

Pull Request Overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-04T23:53:35Z

+        assertEq(tokensWithCollectedFees.length, 0);
+
+        // test withdrawal of profit from the fee contract as app manager
+         FeeUtils.OwnerAttestation memory oa = FeeUtils.OwnerAttestation({


[nitpick] Extra space before FeeUtils.OwnerAttestation creates inconsistent indentation. Should align with surrounding code.

Suggested change

FeeUtils.OwnerAttestation memory oa = FeeUtils.OwnerAttestation({

FeeUtils.OwnerAttestation memory oa = FeeUtils.OwnerAttestation({

Copilot

Pull Request Overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 14 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull Request Overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

packages/libs/contracts-sdk/abis/FeeDiamond.abi.json:741

The ABI for withdrawAppFees is incomplete. The function signature in FeeAdminFacet.sol includes two additional parameters: FeeUtils.OwnerAttestation calldata ownerAttestation and bytes calldata ownerAttestationSig, but these are missing from the ABI. This will cause runtime errors when trying to call this function from off-chain code.

    "name": "withdrawAppFees",
    "inputs": [
      {
        "name": "appId",
        "type": "uint40",
        "internalType": "uint40"
      },
      {
        "name": "tokenAddress",
        "type": "address",
        "internalType": "address"
      }
    ],

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-05T01:30:04Z

  {
    "type": "function",
-    "name": "withdrawFromMorpho",
+    "name": "vincentAppDiamond",


The function name in the ABI is vincentAppDiamond but the actual function in FeeAdminFacet.sol is vincentAppDiamondOnYellowstone(). This naming mismatch will cause function calls to fail. The ABI must match the Solidity function name exactly.

Suggested change

"name": "vincentAppDiamond",

"name": "vincentAppDiamondOnYellowstone",

Copilot · 2025-11-05T01:30:05Z

  {
    "type": "function",
-    "name": "transferOwnership",
+    "name": "setVincentAppDiamond",


The function name in the ABI is setVincentAppDiamond but the actual function in FeeAdminFacet.sol is setVincentAppDiamondOnYellowstone(). This naming mismatch will cause function calls to fail. The ABI must match the Solidity function name exactly.

Suggested change

"name": "setVincentAppDiamond",

"name": "setVincentAppDiamondOnYellowstone",

Copilot · 2025-11-05T01:30:05Z

@@ -1,4 +1,20 @@
 [


The ABI is missing the withdrawPlatformFees function that exists in FeeAdminFacet.sol. This function is critical for the Lit Foundation to withdraw their portion of collected fees and should be included in the ABI.

Copilot · 2025-11-05T01:30:05Z

@@ -1,4 +1,20 @@
 [


The ABI is missing several new view functions added to FeeAdminFacet: litFoundationWallet(), ownerAttestationSigner(), and litAppFeeSplitPercentage() is present but verifyOwnerAttestation() is missing. These should be included for completeness.

Copilot · 2025-11-05T01:30:06Z

+      {
+        "name": "caller",
+        "type": "address",
+        "internalType": "address"


The CallerNotAppManager error definition is incomplete. According to FeeAdminFacet.sol line 24, this error takes three parameters: (uint40 appId, address caller, address recoveredSigner), but the ABI only includes two parameters. The missing recoveredSigner parameter should be added.

Suggested change

"internalType": "address"

"internalType": "address"

},

{

"name": "recoveredSigner",

"type": "address",

"internalType": "address"

Copilot · 2025-11-05T01:30:06Z

@@ -1,4 +1,20 @@
 [


The ABI is missing error definitions for several errors added to FeeAdminFacet.sol: CallerNotLitFoundation, OwnerAttestationIncorrectSigner, OwnerAttestationIncorrectAppId, OwnerAttestationExpired, OwnerAttestationIssuedAtInFuture, OwnerAttestationIncorrectChainId, OwnerAttestationIncorrectDestinationContract, OwnerAttestationIncorrectSourceChainId, and OwnerAttestationIncorrectSourceContract. These should be included for proper error handling in client code.

Copilot · 2025-11-05T01:30:06Z

@@ -1,4 +1,20 @@
 [


The ABI is missing the event definition for LitFoundationWalletSet that is emitted in FeeAdminFacet.sol line 226. This event should be included for off-chain monitoring of configuration changes.

Copilot · 2025-11-05T01:30:06Z

@@ -1,4 +1,20 @@
 [


The ABI is missing the event definition for OwnerAttestationSignerSet that is emitted in FeeAdminFacet.sol line 236. This event should be included for off-chain monitoring of configuration changes.

glitch003 added 4 commits October 14, 2025 09:21

fix: on aerodrome swap, send tokens direct to the user from aerodrome…

c071709

…, since we already took out the fee

feat: morpho where the user always holds the shares, so they can coll…

2b33210

…ect rewards

fix: morpho non fork tests

a511310

feat: aave fix so that the user holds the shares the entire time

bf67b2b

fix: forge fmt

f1fbe29

vercel Bot deployed to Preview October 15, 2025 02:47 View deployment

glitch003 added 4 commits October 15, 2025 14:21

feat: started big refactor to send fees to apps

f85f748

fix: one more aave test working

f2bb19a

fix: add deposit and withdraw events, fix remaining aave tests

34d7b0d

fix: forge fmt

5afacc5

vercel Bot deployed to Preview October 15, 2025 21:35 View deployment

glitch003 added 6 commits October 15, 2025 16:19

fix: remove user token tracking mapping because the user holds the as…

1e9ed7f

…sets now and so they can just look onchain or use walletconnect

fix: aerodrome test

414792c

fix: forge fmt

c51f678

fix: MorphoFee tests

0956f04

fix: morpho fee fork tests

1ecf225

fix: forge fmt

5876a10

vercel Bot deployed to Preview October 16, 2025 01:28 View deployment

fix: docs on fee stuff

2837298

vercel Bot deployed to Preview October 16, 2025 01:35 View deployment

fix: forge fmt with latest stable foundry to match CI

9326237

vercel Bot deployed to Preview October 16, 2025 01:40 View deployment

glitch003 marked this pull request as ready for review October 16, 2025 18:11

glitch003 requested review from Ansonhkg, DashKash54, FedericoAmura and awisniew207 as code owners October 16, 2025 18:11

Copilot AI review requested due to automatic review settings October 16, 2025 18:11

glitch003 requested a review from MaximusHaximus as a code owner October 16, 2025 18:11

Copilot AI reviewed Oct 18, 2025

View reviewed changes

vercel Bot deployed to Preview October 18, 2025 00:10 View deployment

DashKash54 reviewed Oct 19, 2025

View reviewed changes

glitch003 added 8 commits October 31, 2025 17:11

feat: contract compiles with new cross-chain owner oracle functionali…

8c3aefa

…ty - need to fix tests and mocks

fix: Added foundation wallet address for fee collection, contracts co…

c14ae60

…mpile but tests don't

fix: basic test passes

015d635

fix: add src chain id and contract address checks

30b391f

fix: first aave withdrawal test works

b85ecb5

fix: all aave fork tests work

fef3b15

fix: aerodrome tests all work

261d353

fix: morpho non-fork tests work

d5df2f9

vercel Bot deployed to Preview November 4, 2025 23:43 View deployment

fix: all tests pass

d53ec3d

Copilot AI review requested due to automatic review settings November 4, 2025 23:51

Copilot AI reviewed Nov 4, 2025

View reviewed changes

vercel Bot deployed to Preview November 4, 2025 23:54 View deployment

glitch003 added 2 commits November 4, 2025 17:17

fix: update fee docs

676de25

Merge branch 'main' into feat/fee-contract-improvements

7a045ea

Copilot AI review requested due to automatic review settings November 5, 2025 01:19

fix: Remove extra space

152e7ae

Copilot AI reviewed Nov 5, 2025

View reviewed changes

vercel Bot deployed to Preview November 5, 2025 01:22 View deployment

fix: forge fmt

899fcdd

vercel Bot deployed to Preview November 5, 2025 01:25 View deployment

fix: error params order

8db5fec

Copilot AI review requested due to automatic review settings November 5, 2025 01:25

fix: make app id uint40

69296f2

vercel Bot deployed to Preview November 5, 2025 01:29 View deployment

Copilot AI reviewed Nov 5, 2025

View reviewed changes

glitch003 merged commit 316784c into main Nov 5, 2025
5 checks passed

	FeeUtils.OwnerAttestation memory oa = FeeUtils.OwnerAttestation({
	FeeUtils.OwnerAttestation memory oa = FeeUtils.OwnerAttestation({

	"name": "vincentAppDiamond",
	"name": "vincentAppDiamondOnYellowstone",

	"name": "setVincentAppDiamond",
	"name": "setVincentAppDiamondOnYellowstone",

Conversation

glitch003 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Uh oh!

vercel Bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DashKash54 left a comment

Choose a reason for hiding this comment

Uh oh!

DashKash54 Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

glitch003 Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

DashKash54 Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

glitch003 Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

DashKash54 Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

DashKash54 Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DashKash54 Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

glitch003 Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

glitch003 commented Oct 14, 2025 •

edited

Loading

vercel Bot commented Oct 14, 2025 •

edited

Loading

DashKash54 Oct 19, 2025 •

edited

Loading