feat: Immutable Vault & Revenue Splitter added to aTokenVaultFactory #105

donosonaumczuk · 2025-08-22T15:06:41Z

No description provided.

…pilation

…e limit issue

…osit transfer on constructor forced it

…RE2 pointer

…mentation

src/ATokenVaultFactory.sol

DhairyaSethi · 2025-09-24T17:50:59Z

src/ATokenVaultFactory.sol

-                            EXTERNAL FUNCTIONS
-    //////////////////////////////////////////////////////////////*/
+    constructor() {
+        VAULT_CREATION_CODE_SSTORE2_POINTER = SSTORE2.write(LibZip.flzCompress(type(ImmutableATokenVault).creationCode));


was simply defining it as a constant (the full bytes) in this contract leading the code size limit issues?

That's correct. The ImmutableATokenVault initcode itself has a size of 29.82 kB, so it cannot fit as a constant anywhere, I tried all the approaches I could think of, even trying to store the compressed initcode bytes (~15.6 kB) and it always exceeded the limit.

╭----------------------------------------╮ | optimizer-runs = 30_000 | current code | |----------------------------------------+------------------+-------------------+--------------------+---------------------╮ | Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | +==========================================================================================================================+ | ATokenVaultFactory | 12,934 | 43,987 | 11,642 | 5,165 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | ImmutableATokenVault | 21,709 | 29,820 | 2,867 | 19,332 | ╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

What it does work is storing the compressed initcode bytes in some library, and getting them from an external function:

library VaultInitCodeLib { function getCompressedInitCode() external pure returns (bytes memory) { return hex"1c61012060405234801562000011575f80fd..."; // ~15.6k bytes - compressed initcode } }

Then do:

LibZip.flzDecompress(VaultInitCodeLib.getCompressedInitCode())

In that case we are doing a DELEGATECALL to the library instead of the EXTCODECOPY to the SSTORE2 pointer. The gas I think should be similar. I chose the SSTORE2 to avoid having the +15k raw bytes of the compressed initcode in the code, I thought that the SSTORE2 approach would be neater in that sense. But I don't mind switching to that approach if you think it would be a better one.

╭----------------------------------------╮ | optimizer-runs = 30_000 | initcode lib | |----------------------------------------+------------------+-------------------+--------------------+---------------------╮ | Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | +==========================================================================================================================+ | ATokenVaultFactory | 12,959 | 12,988 | 11,617 | 36,164 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | ImmutableATokenVault | 21,709 | 29,820 | 2,867 | 19,332 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | VaultInitCodeLib | 15,891 | 15,944 | 8,685 | 33,208 | ╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

In the next comment I will post some other things I tried in case it's helpful.

Putting the compressed initcode raw bytes directly into the factory:

╭-------------------------------------╮ | optimizer-runs = 30_000 | raw bytes | |----------------------------------------+------------------+-------------------+--------------------+---------------------╮ | Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | +==========================================================================================================================+ | ATokenVaultFactory | 28,368 | 28,397 | -3,792 | 20,755 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | ImmutableATokenVault | 21,709 | 29,820 | 2,867 | 19,332 | ╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

If we lower the optimizer runs down to 1 (which I don't like, but just for the sake of looking at the numbers):

╭--------------------------------╮ | optimizer-runs = 1 | raw bytes | |----------------------------------------+------------------+-------------------+--------------------+---------------------╮ | Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | +==========================================================================================================================+ | ATokenVaultFactory | 26,036 | 26,065 | -1,460 | 23,087 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | ImmutableATokenVault | 16,942 | 24,879 | 7,634 | 24,273 | ╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

That was using always the same compressed initcode that I compiled and compressed when the optimizer runs were set to 30k.
So, even if I lower the optimizer runs down to 1 for the the compressed initcode generation, the factory still exceeds the size limit:

╭------------------------------------------------------------------╮ | optimizer-runs = 1 | raw bytes generated with optimizer-runs = 1 | |----------------------------------------+------------------+-------------------+--------------------+---------------------╮ | Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | +==========================================================================================================================+ | ATokenVaultFactory | 25,923 | 25,952 | -1,347 | 23,200 | |----------------------------------------+------------------+-------------------+--------------------+---------------------| | ImmutableATokenVault | 16,942 | 24,879 | 7,634 | 24,273 | ╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

DhairyaSethi · 2025-09-24T17:51:57Z

src/ATokenVaultFactory.sol

+        bytes32 salt = bytes32(_nextSalt++);
+
+        bytes memory vaultInitCode = abi.encodePacked(
+            LibZip.flzDecompress(SSTORE2.read(VAULT_CREATION_CODE_SSTORE2_POINTER)),


sstore2 would be useful is this data was dynamic but since it's a constant i really don't see the need for this unless code size issues in which case i would try more to decrease size of the factory instead

I explained why the SSTORE2 and InitCode Compression in this comment.

Given the size of the ImmutableATokenVault initcode, it will not fit straight away in the factory. I do not see a clear path on how to get the factory code leaner (except for getting the ATokenVaultRevenueSplitterOwner deployment to a lib, but then I'd rather take the ImmutableATokenVault to a lib as it's the bigger one in bytecode)

I think it comes down to SSTORE2+Compression (no raw bytes in the code) VS InitCodeLib+Compression (raw bytes in the code).

DhairyaSethi · 2025-09-24T17:54:02Z

src/ATokenVaultFactory.sol

+    function _deployVault(bytes memory vaultInitCode, bytes32 salt) internal returns (address) {
+        address vaultAddress;
+        assembly {
+            vaultAddress := create2(0, add(vaultInitCode, 32), mload(vaultInitCode), salt)


let's just use OZ create2 abstraction for this like address computation instead?

The reason why I did not use the OZ deploy function is that they always revert with "Create2: Failed on deploy" and I wanted to bubble-up the original error, for example "ZERO_INITIAL_LOCK_DEPOSIT", "EMPTY_SHARE_SYMBOL", etc

DhairyaSethi · 2025-09-24T17:55:36Z

src/ATokenVaultFactory.sol

can we rename to immutableATokenVault
we can maintain the factory for both variants

same for tests, is the plan to not maintain both now?

The plan is to have a single factory. The previous factory impl was in practice also an "Immutable aToken Vault", it's just that immutability was achieved by putting a renounced proxy admin instead

DhairyaSethi · 2025-09-24T17:56:44Z

src/ATokenVaultFactory.sol

+        address indexed revenueSplitterOwner,
+        address indexed vault,
+        address indexed owner,
+        ATokenVaultRevenueSplitterOwner.Recipient[] revenueRecipients


let's reference from interface?

Hmm there is no interface for this file. I can create one and use it, but given we are deploying that contract in the factory, it will not change anything in terms of things to import into the factory. Lmk

DhairyaSethi · 2025-09-24T17:58:19Z

script/DeployFactoryImplementation.s.sol

+
+        console.log("Deploying aTokenVaultFactory implementation...");
+
+        ATokenVaultFactory factoryImplementation = new ATokenVaultFactory();


would rm implementation suffix from this file everywhere since its not a proxy it's slight confusing

This is meant to be used to deploy the impl so I can upgrade current factories. I can rename it anyways

Maybe this renaming?

DeployFactory.s.sol -------------------> DeployFactoryWithProxy.s.sol
DeployFactoryImplementation.s.sol ---> DeployFactory.s.sol

I can also just delete the file from the repo as I will use it only once for these upgrades

DhairyaSethi · 2025-09-24T17:58:38Z

src/ATokenVaultFactory.sol

same for tests, is the plan to not maintain both now?

DhairyaSethi · 2025-09-24T18:00:40Z

src/ATokenVaultFactory.sol

+        ATokenVaultRevenueSplitterOwner.Recipient[] memory revenueRecipients
+    ) internal returns (address) {
+        address revenueSplitter = address(new ATokenVaultRevenueSplitterOwner(vaultAddress, owner, revenueRecipients));
+        emit RevenueSplitterOwnerDeployed(revenueSplitter, vaultAddress, owner, revenueRecipients);


shouldn't the constructor params of this contract be emitted in that contract instead?

The deployed contract emits the relevant events, I think this is needed for the backend to make a quicker indexing, but I am happy to just emit the deployed address in the factory event and not the constructor params, cc-ing backend team to confirm @joshstevens19 @desfero

To make sure we only index well known ATokenVaultRevenueSplitterOwner contract event needs to come from an approved factory address (indexer filters if event.tx_information.address is an approved factory contract).
If the event is emitted inside the contract we can't understand quickly if it has proper shape/logic.

What about the emit RevenueSplitterOwnerDeployed(revenueSplitterAddress) vs emit RevenueSplitterOwnerDeployed(revenueSplitter, constructorArgument1, constructorArgument2, etc)

we need all details to index the data without RPC lookup, otherwise indexing is 1000x slower and less predictable (network issues, RPC rate limiting, etc)

github-actions · 2025-09-25T23:49:06Z

Changes to gas cost

Generated at commit: 9f0fcbd5aa59cf1e6eee07a6f96bdc1af3b61665, compared to commit: 497d18da638e6d6f5899f560449f564111e480e1

🧾 Summary (20% most significant diffs)

Contract	Method	Avg (+/-)	%
ATokenVault	initialize	+96,982 ❌	+25.59%

Full diff report 👇

Contract	Deployment Cost (+/-)	Method	Min (+/-)	%	Avg (+/-)	%	Median (+/-)	%	Max (+/-)	%	# Calls (+/-)
ATokenVault	4,799,278 (-188)	convertToAssets convertToShares deposit initialize mint previewDeposit previewWithdraw redeem totalAssets withdraw	11,709 (0) 2,590 (0) 5,288 (0) 3,730 (0) 7,532 (0) 22,561 (0) 18,811 (0) 10,990 (0) 9,052 (0) 11,035 (0)	0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00%	22,088 (-217) 12,234 (+79) 157,030 (-133) 476,018 (+96,982) 113,309 (+332) 23,770 (+144) 27,832 (+71) 107,366 (+13) 10,886 (-70) 93,242 (+212)	-0.97% +0.65% -0.08% +25.59% +0.29% +0.61% +0.26% +0.01% -0.64% +0.23%	15,034 (0) 11,714 (0) 151,615 (0) 442,708 (+38,500) 112,721 (0) 22,561 (0) 27,976 (0) 75,173 (0) 9,052 (0) 75,238 (0)	0.00% 0.00% 0.00% +9.52% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00%	38,026 (0) 37,131 (0) 308,158 (0) 585,842 (0) 265,170 (0) 25,886 (0) 27,976 (0) 248,411 (0) 35,369 (0) 248,548 (0)	0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00% 0.00%	3,135 (-66) 1,546 (0) 30,111 (-66) 239 (-1,298) 1,539 (0) 256 (0) 256 (0) 2,337 (0) 262 (0) 2,069 (0)

donosonaumczuk · 2025-09-26T20:29:03Z

Closed by #108 given that the approach to not exceed code size limit is very unusual

donosonaumczuk added 11 commits August 22, 2025 12:03

misc: Temporaily exclude tests and scripts involving factory from com…

d2200c2

…pilation

misc: Style change in solmate remapping

286b6e6

feat: Immutable Vault & Revenue Splitter added to the factory

c160b57

feat: Use SSTORE2 to store/load vault's creation code to fix code siz…

9761bb6

…e limit issue

test: Existing factory tests adapted

6c9643f

feat: Deploy vault through CREATE2 instead of CREATE3 - Init lock dep…

251e2ec

…osit transfer on constructor forced it

misc: Add solady to remappings

f55cbc5

feat: Use FastLZ compression to fit vault creation code into the SSTO…

69a0b8e

…RE2 pointer

test: Add size limit test for the SSTORE2 pointer

bc6e7e9

misc: Deployment script both for factory and for its standalone imple…

f077284

…mentation

misc: Put back the address(0) placeholder in script

b33107d

donosonaumczuk marked this pull request as ready for review August 27, 2025 17:23

donosonaumczuk requested review from foodaka and miguelmtzinf as code owners August 27, 2025 17:23

donosonaumczuk added 2 commits September 1, 2025 19:34

misc: Merge main and solve conflicts

b6adc10

test: Tests added for Splitter in the Factory

fd26009

DhairyaSethi reviewed Sep 24, 2025

View reviewed changes

misc: Rename _nextSalt to _nextSaltNonce

bb24acb

donosonaumczuk mentioned this pull request Sep 26, 2025

feat: Revenue Splitter added to aTokenVault Factory #108

Open

donosonaumczuk closed this Sep 26, 2025

rhmhsydjabr0-ctrl mentioned this pull request Nov 6, 2025

Create Osama2535 #110

Open


		console.log("Deploying aTokenVaultFactory implementation...");

		ATokenVaultFactory factoryImplementation = new ATokenVaultFactory();

feat: Immutable Vault & Revenue Splitter added to aTokenVaultFactory #105

feat: Immutable Vault & Revenue Splitter added to aTokenVaultFactory #105

Uh oh!

Conversation

donosonaumczuk commented Aug 22, 2025

Uh oh!

Uh oh!

DhairyaSethi Sep 24, 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!

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!

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!

github-actions bot commented Sep 25, 2025

Changes to gas cost

🧾 Summary (20% most significant diffs)

Uh oh!

donosonaumczuk commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DhairyaSethi Sep 24, 2025 •

edited

Loading