Skip to content

Conversation

bxmmm1
Copy link
Collaborator

@bxmmm1 bxmmm1 commented Jun 16, 2025

No description provided.

);

calldatas[calldataIndex++] =
abi.encodeCall(AccessManager.grantRole, (ORACLE_ROLE_ID, accessManagerConfiguration.admin, 0));

Choose a reason for hiding this comment

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

oracle role to admin? also it is not assigned to any function. Will it be used externally?

Choose a reason for hiding this comment

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

I think it should be assigned to the setValidatorsETH but it's not part of the interface

console.log("Vault:", accessManagerConfiguration.vault);
console.logBytes(customExternalCallData);

// TODO: Custom external call directly to the beacon deposit contract

Choose a reason for hiding this comment

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

Can we remove all the commented code and TODO?

@eladiosch
Copy link

May I suggest having a roles-config-files dir and having a file per institution (like matrix.json)? And passing the path to the CustomExternalCallNonRestakingValidators and InitialAccessManagerSetup scripts. If we plan on creating vaults for several institutions might be tidier

@eladiosch
Copy link

Also a proper README file would be useful

Comment on lines +15 to +20
uint64 public constant ADMIN_ROLE_ID = type(uint64).min; // 0
uint64 public constant DEPOSITOR_ROLE_ID = 1;
uint64 public constant WITHDRAWER_ROLE_ID = 2;
uint64 public constant CUSTOM_EXTERNAL_CALLER_ROLE_ID = 3;
uint64 public constant WITHDRAWAL_MANAGER_ROLE_ID = 4;
uint64 public constant ORACLE_ROLE_ID = 5;

Choose a reason for hiding this comment

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

maybe move the roles to its own file to make it more manageable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants