Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
ds-test/=lib/solmate/lib/ds-test/src/
erc4626-tests/=lib/erc4626-tests/
forge-std/=lib/forge-std/src/
solmate/=lib/solmate/src/
@solmate/=lib/solmate/src/
@openzeppelin-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/
@openzeppelin-v5/=lib/openzeppelin-contracts-v5.3.0/contracts
@openzeppelin/=lib/openzeppelin-contracts/contracts
@pcaversaccio/createx/=lib/createx/src/
@solady/=lib/createx/lib/solady/src/
14 changes: 1 addition & 13 deletions script/DeployFactory.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,11 @@ contract DeployFactory is Script {

vm.startBroadcast();


/////// Deploy Renounced ProxyAdmin

console.log("Deploying vault's renounced proxy admin");

address renouncedProxyAdmin = address(new ProxyAdmin());

console.log("Renounced proxy admin deployed at: ", renouncedProxyAdmin);

ProxyAdmin(renouncedProxyAdmin).renounceOwnership();


/////// Deploy aTokenVaultFactory Implementation (pass Renounced ProxyAdmin as argument)

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

ATokenVaultFactory factoryImplementation = new ATokenVaultFactory({proxyAdmin: renouncedProxyAdmin});
ATokenVaultFactory factoryImplementation = new ATokenVaultFactory();

console.log("aTokenVaultFactory implementation deployed at: ", address(factoryImplementation));

Expand Down
36 changes: 36 additions & 0 deletions script/DeployFactoryImplementation.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.10;

import "forge-std/Script.sol";
import {ATokenVaultFactory} from "../src/ATokenVaultFactory.sol";

/**
* @title DeployFactoryImplementation
* @author Aave Labs
* @notice Script to deploy the aTokenVaultFactory contract implementation
* @dev Run the script with the following command first:
*
* forge script script/DeployFactoryImplementation.s.sol:DeployFactoryImplementation -vvvv --rpc-url {$RPC_URL} --account ${ACCOUNT} --slow
*
* If succeeds, then add the --broadcast flag in order to send the transaction to the network.
*/
contract DeployFactoryImplementation is Script {

function run() external {

console.log("BlockNumber: ", block.number);

console.log("ChainId: ", block.chainid);

vm.startBroadcast();

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

ATokenVaultFactory factoryImplementation = new ATokenVaultFactory();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this renaming?

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


console.log("aTokenVaultFactory implementation deployed at: ", address(factoryImplementation));

vm.stopBroadcast();
}
}
189 changes: 104 additions & 85 deletions src/ATokenVaultFactory.sol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,26 @@ pragma solidity ^0.8.10;

import {IERC20} from "@openzeppelin/interfaces/IERC20.sol";
import {IPoolAddressesProvider} from "@aave-v3-core/interfaces/IPoolAddressesProvider.sol";
import {TransparentUpgradeableProxy} from "@openzeppelin/proxy/transparent/TransparentUpgradeableProxy.sol";
import {ATokenVault} from "./ATokenVault.sol";
import {SafeERC20} from "@openzeppelin/token/ERC20/utils/SafeERC20.sol";
import {ProxyAdmin} from "@openzeppelin/proxy/transparent/ProxyAdmin.sol";
import {ATokenVaultRevenueSplitterOwner} from "./ATokenVaultRevenueSplitterOwner.sol";
import {ImmutableATokenVault} from "./ImmutableATokenVault.sol";
import {SSTORE2} from "@solmate/utils/SSTORE2.sol";
import {Create2} from "@openzeppelin/utils/Create2.sol";
import {LibZip} from "@solady/utils/LibZip.sol";

/**
* @title ATokenVaultImplDeploymentLib
* @author Aave Labs
* @notice Library that handles the deployment of the ATokenVault implementation contract
* @dev This library is a helper to avoid holding the ATokenVault bytecode in the factory contract avoiding exceeding
* the contract size limit.
* @dev Struct containing constructor parameters for vault deployment
*/
library ATokenVaultImplDeploymentLib {
function deployVaultImpl(
address underlying,
uint16 referralCode,
IPoolAddressesProvider poolAddressesProvider
) external returns (address vault) {
return address(new ATokenVault(
underlying,
referralCode,
poolAddressesProvider
));
}
struct VaultParams {
address underlying;
uint16 referralCode;
IPoolAddressesProvider poolAddressesProvider;
address owner;
uint256 initialFee;
string shareName;
string shareSymbol;
uint256 initialLockDeposit;
ATokenVaultRevenueSplitterOwner.Recipient[] revenueRecipients;
}

/**
Expand All @@ -39,75 +35,48 @@ library ATokenVaultImplDeploymentLib {
contract ATokenVaultFactory {
using SafeERC20 for IERC20;

/*//////////////////////////////////////////////////////////////
EVENTS
//////////////////////////////////////////////////////////////*/

/**
* @dev Emitted when a new vault is deployed
* @param vault The address of the deployed vault proxy
* @param implementation The address of the vault implementation
* @param vault The address of the deployed vault
* @param underlying The underlying asset address
* @param deployer The address that deployed the vault
* @param params The parameters used to deploy the vault
*/
event VaultDeployed(
address indexed vault,
address indexed implementation,
address indexed underlying,
address deployer,
VaultParams params
);

/*//////////////////////////////////////////////////////////////
CONSTANTS
//////////////////////////////////////////////////////////////*/

/// @notice Proxy admin address for all deployed vaults, with renounced ownership.
/// @dev Future version will deploy a plain immutable vault without proxy.
address internal immutable RENOUNCED_PROXY_ADMIN;

/*//////////////////////////////////////////////////////////////
STRUCTS
//////////////////////////////////////////////////////////////*/

/**
* @dev Struct containing constructor parameters for vault deployment
* @dev Emitted when a new revenue splitter owner is deployed
* @param revenueSplitterOwner The address of the deployed revenue splitter owner
* @param vault The address of the vault to split the revenue from
* @param owner The address of the owner of the revenue splitter, effective owner of the vault
* @param revenueRecipients The recipients of the revenue
*/
struct VaultParams {
address underlying;
uint16 referralCode;
IPoolAddressesProvider poolAddressesProvider;
address owner;
uint256 initialFee;
string shareName;
string shareSymbol;
uint256 initialLockDeposit;
}
event RevenueSplitterOwnerDeployed(
address indexed revenueSplitterOwner,
address indexed vault,
address indexed owner,
ATokenVaultRevenueSplitterOwner.Recipient[] revenueRecipients
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's reference from interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

);

/*//////////////////////////////////////////////////////////////
CONSTRUCTOR
//////////////////////////////////////////////////////////////*/
address immutable public VAULT_CREATION_CODE_SSTORE2_POINTER;

/**
* @dev Constructor
* @param proxyAdmin The address that will be the admin of all deployed proxies. Must have renounced ownership.
*/
constructor(address proxyAdmin) {
RENOUNCED_PROXY_ADMIN = proxyAdmin;
require(ProxyAdmin(proxyAdmin).owner() == address(0), "PROXY_ADMIN_OWNERSHIP_NOT_RENOUNCED");
}
uint256 internal _nextSaltNonce;

/*//////////////////////////////////////////////////////////////
EXTERNAL FUNCTIONS
//////////////////////////////////////////////////////////////*/
constructor() {
VAULT_CREATION_CODE_SSTORE2_POINTER = SSTORE2.write(LibZip.flzCompress(type(ImmutableATokenVault).creationCode));
Copy link
Member

@DhairyaSethi DhairyaSethi Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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              |
╰----------------------------------------+------------------+-------------------+--------------------+---------------------╯

}

/**
* @notice Deploy a new ATokenVault with the given parameters
* @notice Deploys a new ATokenVault with the given parameters
* @param params All parameters needed for vault deployment and initialization
* @return vault The address of the deployed vault proxy
*/
function deployVault(VaultParams memory params) public returns (address vault) {
function deployVault(VaultParams memory params) public returns (address) {
require(params.underlying != address(0), "ZERO_ADDRESS_NOT_VALID");
require(address(params.poolAddressesProvider) != address(0), "ZERO_ADDRESS_NOT_VALID");
require(params.owner != address(0), "ZERO_ADDRESS_NOT_VALID");
Expand All @@ -121,34 +90,84 @@ contract ATokenVaultFactory {
params.initialLockDeposit
);

address implementation = ATokenVaultImplDeploymentLib.deployVaultImpl(
params.underlying,
params.referralCode,
params.poolAddressesProvider
bytes32 salt = bytes32(_nextSaltNonce++);

bytes memory vaultInitCode = abi.encodePacked(
LibZip.flzDecompress(SSTORE2.read(VAULT_CREATION_CODE_SSTORE2_POINTER)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

abi.encode(
params.underlying,
params.referralCode,
params.poolAddressesProvider,
address(this),
params.initialFee,
params.shareName,
params.shareSymbol,
params.initialLockDeposit
)
);

vault = address(new TransparentUpgradeableProxy(
implementation,
RENOUNCED_PROXY_ADMIN,
""
));
address vaultAddress = _computeVaultAddress(vaultInitCode, salt);

IERC20(params.underlying).safeApprove(vault, params.initialLockDeposit);
IERC20(params.underlying).safeApprove(vaultAddress, params.initialLockDeposit);

ATokenVault(vault).initialize(
params.owner,
params.initialFee,
params.shareName,
params.shareSymbol,
params.initialLockDeposit
);
_deployVault(vaultInitCode, salt);

address owner = params.owner;
if (params.revenueRecipients.length > 0) {
owner = _deployRevenueSplitterOwner(vaultAddress, params.owner, params.revenueRecipients);
}
ImmutableATokenVault(vaultAddress).transferOwnership(owner);

emit VaultDeployed(
vault,
implementation,
vaultAddress,
params.underlying,
msg.sender,
params
);

return vaultAddress;
}

/**
* @notice Deploys a new ATokenVaultRevenueSplitterOwner with the given parameters
* @param vaultAddress The address of the vault to split the revenue from
* @param owner The address of the owner of the revenue splitter, effective owner of the vault
* @param revenueRecipients The recipients of the revenue
* @return revenueSplitter The address of the deployed revenue splitter
*/
function deployRevenueSplitterOwner(
address vaultAddress,
address owner,
ATokenVaultRevenueSplitterOwner.Recipient[] memory revenueRecipients
) external returns (address) {
return _deployRevenueSplitterOwner(vaultAddress, owner, revenueRecipients);
}

function _deployRevenueSplitterOwner(
address vaultAddress,
address owner,
ATokenVaultRevenueSplitterOwner.Recipient[] memory revenueRecipients
) internal returns (address) {
address revenueSplitter = address(new ATokenVaultRevenueSplitterOwner(vaultAddress, owner, revenueRecipients));
emit RevenueSplitterOwnerDeployed(revenueSplitter, vaultAddress, owner, revenueRecipients);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

return revenueSplitter;
}

function _computeVaultAddress(bytes memory vaultInitCode, bytes32 salt) internal view returns (address) {
return Create2.computeAddress(salt, keccak256(vaultInitCode));
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// If the deployment fails, revert bubbling up the error
if iszero(vaultAddress) {
let returnDataSize := returndatasize()
returndatacopy(0, 0, returnDataSize)
revert(0, returnDataSize)
}
}
return vaultAddress;
}
}
Loading