feat: poc of new FeeVault & FeeVaultInitializer implementations

[funkornaut001]

Fee Vault Architecture Refactor and Upgradeability
Refactored the core FeeVault contract to use storage variables (recipient, minWithdrawalAmount, withdrawalNetwork) instead of immutable constructor parameters, added OpenZeppelin's Initializable and proxy admin ownership, and provided setter functions with corresponding update events for these parameters.

Migration Support
Added a new contract, FeeVaultInitializer, which migrates legacy fee vaults to the new upgradeable contracts by withdrawing funds, deploying new implementations, and upgrading proxies with initialization calls.

[tynes]

Thanks for starting on this! I just added some new commits to the design doc to try to clarify things: ethereum-optimism/design-docs#309

Let me know if you have any questions!

[tynes]

I am not sure if this is going to work exactly because it needs to be the proxy admin or address(0) that makes this call, see https://github.com/ethereum-optimism/optimism/blob/7c9586773f980266cce7235f38cc6ee7295a8870/packages/contracts-bedrock/src/universal/Proxy.sol#L75

What we usually do is have a network upgrade transaction that spoofs address(0) to make the initialize call. You can see an example here: https://github.com/ethereum-optimism/optimism/blob/7c9586773f980266cce7235f38cc6ee7295a8870/op-node/rollup/derive/isthmus_upgrade_transactions.go#L105

[tynes]

I was thinking there has to be some way that its possible to thread the information to the right place. Looking at the code now, the easiest thing to do would be to just keep the values as immutables because we could read the values, deploy the fee vaults with immutables and then follow up with a transaction that spoofs address(0) and calls the proxy upgradeToAndCall

Moving away from immutables is just an optimization for the runbook, if its impossible to migrate to them being in storage that is ok

[0xDiscotech]

Thanks for the feedback — agreed, this won’t work as-is.

If we still want to have the migration logic (getting current values and set them on the newly deployed vault's) on the FeeVaultInitializer, what we can do is setting them as immutables but adding this change on the vaults to keep enabling the setter logic:

address internal immutable _RECIPIENT;

address internal _recipient;

...

  function recipient() external view returns (address) {
    if (_recipient == address(0) {
      // If the storage var not set, use the immutable
      return _RECIPIENT;
    }
     // If set, use the storage var
    return _recipient;
  }

Even though this is less efficient, it achieves the goal of having the values set by the FeeVaultInitializer while still allowing the proxy admin to change them later. The NUTs would be:

• Call migrate() on the FeeVaultInitializer.
• Call upgradeTo() from the address(0) for each Vault to point to the new implementation.

Does this direction make sense? Happy to keep exploring alternatives if not

[0xDiscotech]

@tynes
We believe that we’ve got a better solution that avoids the workaround above. We can store the current configs as immutables on the FeeVaultInitializer contract, grant the initializer permission on the vaults to call initialize(), and then upgrade each vault proxy to point to the new implementation.

NUTs batch:

1. Deploy the FeeVaultInitializer. In the constructor, it reads the current config values from the live vaults and stores them as immutables.
2. Deploy the implementation for each vault, using the deployed initializer (pre-calculated) as the allowed initializer address (1 NUT per vault). *
3. From address(0), call upgradeTo for each vault proxy to point to the new implementation (1 NUT per vault).
4. Call FeeVaultInitializer.initializeVaults() so it initializes all vault configs based on its immutables.

This gives us a clear runbook while keeping changes on the vault side minimal, we’d only need something like this:

address public immutable FEE_VAULT_INITIALIZER;
Config public config;

constructor(address _feeVaultInitializer) {
  FEE_VAULT_INITIALIZER = _feeVaultInitializer;
}

function initialize(Config _config) external {
  if (msg.sender != FEE_VAULT_INITIALIZER) revert NotAllowed();
  config = _config;
}

*Step 2 could also be automated inside FeeVaultInitializer, but that adds more logic/complexity and increases contract size.

Does this approach feel on the right track?

[tynes]

This is generally looking good!