Skip to content

Update to the vault script#109

Closed
Streator wants to merge 17 commits intomainfrom
script-updates
Closed

Update to the vault script#109
Streator wants to merge 17 commits intomainfrom
script-updates

Conversation

@Streator
Copy link
Copy Markdown
Contributor

@Streator Streator commented Sep 9, 2025

PR Type

Enhancement


Description

  • Add comprehensive vault deployment scripts for Symbiotic protocol

  • Support both regular and tokenized vault deployments

  • Include role-based access control validation

  • Provide configurable parameters for delegators and slashers


Diagram Walkthrough

flowchart LR
  A["DeployVault.s.sol"] --> B["DeployVaultBase.sol"]
  C["DeployVaultTokenized.s.sol"] --> D["DeployVaultTokenizedBase.sol"]
  D --> B
  B --> E["Logs.sol"]
  B --> F["Vault Creation"]
  F --> G["Role Validation"]
Loading

File Walkthrough

Relevant files
Enhancement
DeployVault.s.sol
Regular vault deployment script implementation                     

script/DeployVault.s.sol

  • Defines configuration constants for vault deployment
  • Implements run() function calling base deployment logic
  • Supports multiple delegator and slasher types
  • Includes optional parameters for deposit limits and whitelisting
+91/-0   
DeployVaultTokenized.s.sol
Tokenized vault deployment script implementation                 

script/DeployVaultTokenized.s.sol

  • Extends regular vault deployment with ERC20 tokenization
  • Adds NAME and SYMBOL constants for token metadata
  • Inherits all configuration options from base vault
  • Calls tokenized base deployment logic
+99/-0   
DeployVaultBase.sol
Base vault deployment logic with validation                           

script/base/DeployVaultBase.sol

  • Core deployment logic with parameter encoding
  • Comprehensive role validation after deployment
  • Support for 4 delegator types and 2 slasher types
  • Handles whitelist depositor setup and role transfers
+356/-0 
DeployVaultTokenizedBase.sol
Tokenized vault base deployment extension                               

script/base/DeployVaultTokenizedBase.sol

  • Extends base deployment for tokenized vaults
  • Overrides vault version to 2 for tokenized implementation
  • Encodes additional name and symbol parameters
  • Maintains all base functionality with token support
+43/-0   
Miscellaneous
Logs.sol
Logging utility for deployment scripts                                     

script/base/Logs.sol

  • Utility contract for logging deployment information
  • Writes logs to both console and file system
  • Provides centralized logging functionality
+16/-0   

@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Insecure logging path

Description: The logging function writes to a fixed relative path 'script/logs.txt' by reading and
concatenating existing file content, which can lead to large unbounded files and potential
workspace poisoning if the file is user-controlled; prefer append operations or bounded
logs.
Logs.sol [10-15]

Referred Code
function log(
    string memory data
) internal {
    console2.log(data);
    vm.writeFile(LOG_FILE, string.concat(vm.readFile(LOG_FILE), data, "\n"));
}
Privilege persistence risk

Description: During deployment with a whitelist, admin and whitelist roles are temporarily assigned to
the deployer and later transferred, but the code relies on vm.readCallers and assertions
rather than enforcing on-chain revocation in all paths, risking lingering privileges if
execution is interrupted before role renouncement.
DeployVaultBase.sol [181-189]

Referred Code
    IVault.InitParams memory baseParams = params.vaultParams.baseParams;
    baseParams.defaultAdminRoleHolder =
        needWhitelistDepositors ? deployer : params.vaultParams.baseParams.defaultAdminRoleHolder;
    baseParams.depositorWhitelistRoleHolder =
        needWhitelistDepositors ? deployer : params.vaultParams.baseParams.depositorWhitelistRoleHolder;

    return abi.encode(baseParams);
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consolidate configuration to reduce duplication

To improve maintainability and prevent inconsistencies, consolidate the
duplicated configuration variables from DeployVault.s.sol and
DeployVaultTokenized.s.sol into a shared base contract.

Examples:

script/DeployVault.s.sol [6-54]
contract DeployVaultScript is DeployVaultBase {
    // Configurations - UPDATE THESE BEFORE DEPLOYMENT

    // Address of the owner of the vault who can migrate the vault to new versions whitelisted by Symbiotic
    address OWNER = 0x0000000000000000000000000000000000000000;
    // Address of the collateral token
    address COLLATERAL = 0x0000000000000000000000000000000000000000;
    // Vault's burner to send slashed funds to (e.g., 0xdEaD or some unwrapper contract; not used in case of no slasher)
    address BURNER = 0x000000000000000000000000000000000000dEaD;
    // Duration of the vault epoch (the withdrawal delay for staker varies from EPOCH_DURATION to 2 * EPOCH_DURATION depending on when the withdrawal is requested)

 ... (clipped 39 lines)
script/DeployVaultTokenized.s.sol [6-58]
contract DeployVaultTokenizedScript is DeployVaultTokenizedBase {
    // Configurations - UPDATE THESE BEFORE DEPLOYMENT

    // Name of the ERC20 representing shares of the active stake in the vault
    string NAME = "SymVault";
    // Symbol of the ERC20 representing shares of the active stake in the vault
    string SYMBOL = "SV";
    // Address of the owner of the vault who can migrate the vault to new versions whitelisted by Symbiotic
    address OWNER = 0x0000000000000000000000000000000000000000;
    // Address of the collateral token

 ... (clipped 43 lines)

Solution Walkthrough:

Before:

// file: script/DeployVault.s.sol
contract DeployVaultScript is DeployVaultBase {
    // Configurations - UPDATE THESE BEFORE DEPLOYMENT
    address OWNER = 0x...;
    address COLLATERAL = 0x...;
    address BURNER = 0x...;
    uint48 EPOCH_DURATION = 7 days;
    // ... many more duplicated config variables
    
    function run() public { ... }
}

// file: script/DeployVaultTokenized.s.sol
contract DeployVaultTokenizedScript is DeployVaultTokenizedBase {
    // Configurations - UPDATE THESE BEFORE DEPLOYMENT
    string NAME = "SymVault";
    string SYMBOL = "SV";
    address OWNER = 0x...;
    address COLLATERAL = 0x...;
    address BURNER = 0x...;
    uint48 EPOCH_DURATION = 7 days;
    // ... many more duplicated config variables

    function run() public { ... }
}

After:

// file: script/config/DeployVaultConfig.sol (new file)
abstract contract DeployVaultConfig {
    // Configurations - UPDATE THESE BEFORE DEPLOYMENT
    address OWNER = 0x...;
    address COLLATERAL = 0x...;
    address BURNER = 0x...;
    uint48 EPOCH_DURATION = 7 days;
    // ... all common config variables
}

// file: script/DeployVault.s.sol
contract DeployVaultScript is DeployVaultBase, DeployVaultConfig {
    function run() public { ... }
}

// file: script/DeployVaultTokenized.s.sol
contract DeployVaultTokenizedScript is DeployVaultTokenizedBase, DeployVaultConfig {
    string NAME = "SymVault";
    string SYMBOL = "SV";
    function run() public { ... }
}
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies significant code duplication in configuration across DeployVault.s.sol and DeployVaultTokenized.s.sol, and proposing a base contract is a good architectural improvement for maintainability.

Medium
Possible issue
Add missing role validation checks

Add a missing validation check in _validateOwnershipTransfer to verify that
NETWORK_LIMIT_SET_ROLE has been correctly granted for the
OperatorSpecificDelegator.

script/base/DeployVaultBase.sol [318-329]

 if (params.delegatorParams.baseParams.hookSetRoleHolder != address(0)) {
     assert(
         OperatorSpecificDelegator(delegator).hasRole(
             OperatorSpecificDelegator(delegator).HOOK_SET_ROLE(),
             params.delegatorParams.baseParams.hookSetRoleHolder
         ) == true
     );
 }
+for (uint256 i = 0; i < params.delegatorParams.networkAllocationSettersOrNetwork.length; ++i) {
+    assert(
+        OperatorSpecificDelegator(delegator).hasRole(
+            OperatorSpecificDelegator(delegator).NETWORK_LIMIT_SET_ROLE(),
+            params.delegatorParams.networkAllocationSettersOrNetwork[i]
+        ) == true
+    );
+}
 assert(
     OperatorSpecificDelegator(delegator).operator()
         == params.delegatorParams.operatorAllocationSettersOrOperator[0]
 );
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a missing validation check for NETWORK_LIMIT_SET_ROLE in the deployment script, which improves the script's robustness by ensuring roles are set as expected.

Medium
  • More

@1kresh 1kresh closed this Oct 24, 2025
@1kresh 1kresh deleted the script-updates branch December 8, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants