Consider fee accrual as 1155 balance

[hensha256]

trafficstars

Components

Singleton, 1155 Balances

Describe the suggested feature and problem it solves.

Currently fees accrued to the protocol and to hooks are stored in mappings. However the ERC1155 implementation of the PoolManager should allow us to actually accrue these as 1155 minting instead of having separate mappings.

Describe the desired implementation.

Remove the 2 mappings protocolFeesAccrued and hookFeesAccrued and instead mint 1155s, similarly to in PoolManager.mint

Describe alternatives.

No response

Additional context.

No response

[hensha256]

Currently protocolFees can be claimed by 2 parties: owner and protocolFeeController this would only be able to mint balance to 1 of those parties (although approval could be given to the other). So we'd have to choose who should have control over protocol fees

[abhithory]

hi @hensha256 , i want to work on this issue. can you please assign it to me?

[hensha256]

Sure! Feel free to tag me here if you have any questions!

[abhithory]

ok. Thank you

[abhithory]

Hi @hensha256 ,

From reading this and checking contracts I am able to understand that:

• currently, we are storing accrued fees from the protocol and hooks in mappings (protocolFeesAccrued and hookFeesAccrued)

"accrued fees" - refer to the fees that have accumulated or been earned by the protocol and hooks.

• But in PoolManager Erc115 is already implemented. So we should directly mint erc115 nft for this accrued fee instead of storing them in mappings.

Am I thinking right?? or is there anything else?

Also if it is right then I am thinking to solve it this way:---

1. Remove mappings (protocolFeesAccrued and hookFeesAccrued)

2. Find all places where fees are added in these mappings

3. then instead of adding fees in mapping directly mint nft with it

also please tell me is it right way to approach?

[abhithory]

Hi @hensha256 , can you please tell me these things??

So now for minting ERC1155, it is required: address to mint, nft id, amount.

so I can set the amount to 1.

what should i set nft id?? currency key? (key.currency0)

and then what to whom should i mint this nft?? to this contract?? (address(this)) or to someone else??

in "collectProtocolFees" function from where should I subtract the amount??

should i create a new mapping for both of the two?? in which we will maintain fees accrued of and then subtract in this function??

and also what should be the key for this mapping?? nft id or same as this??

[hensha256]

Hi @abhithory!

Your approach in your first comment sounds correct to me! Please feel free to try implementing that as a PR!

what should i set nft is?? currency key? (key.currency0)

If you look at the mint function of the PoolManager you can see how we mint 1155s for currencies.

to whom should i mint this nft

For hooks, the hook itself should own the 1155. As you can see in collectHookFees, the hook address owns its own fees.

For protocol fees it will need to either be the protocolFeeController or the owner. Currently in collectProtocolFees, both of these addresses have permission to collect protocol fees. We will only be able to have 1 address owning the 1155, so we will have to choose 1 address. For now, mint it to the owner, but we may change it in future.

[hensha256]

in "collectProtocolFees" function from where should I subtract the amount??

The amount will be the NFT owner's balance. For that reason we will not need the functions collect{Protocol,Hook}Fees anymore. They can be removed. To collect fees, the protocol/hook will burn 1155s in the same way that users will.

[abhithory]

@hensha256 thank you very much.... I will make its correct PR.

[abhithory]

hi @hensha256 , when i remove protocolFeesAccrued mapping then it shows error in PoolManager contract because it is overridden from IPoolManager interface. so i have removed it from IPoolManager also....

is it okay??

[abhithory]

Hi @hensha256 , I apologize for asking so many questions, but this is my first time contributing, and I want to make sure I don't make any mistakes. That's why I'm asking about everything.

So, in the PoolManager contract, I have correctly implemented ERC1155 for minting fee accrual. However, after removing the old functions, some test cases are failing in both Hardhat and Foundry tests. So, i commented these function uses in test files and then run all tests according to contribute.md documentation. and everything else is fine after commenting.

Now, what should I do?

• only commit PoolManager contract and make PR of it? ( and then again in new PR i can write testcasess also. where i think i have to check balance of user of nft and then run tests)
• commit PoolManager and commented testcases files and then make PR.
• make test files correct right now with all testcases cases and then commit PoolManager and testfiles and then make PR.

Please let me know what will be more suitable.

[abhithory]

hi @hensha256 , please let me know at least.... am i asking extra? or anything wrong?

[Arveymenon]

Hi, My two cents on it would be that the test cases do need to be updated in this case. Need to check for 1155s being mint

[kevin-fruitful]

Hi, I gave it a shot, please let me know if there's anything I should change, thanks!

[hensha256]

We have decided not to move ahead with this for now due to the additional cost on swaps for minting a 6909