Skip to content

Conversation

@chechu
Copy link
Contributor

@chechu chechu commented Dec 18, 2024

Description

Resolves VEN-2882

@web3rover web3rover marked this pull request as ready for review December 18, 2024 11:23
web3rover and others added 3 commits December 19, 2024 14:30
@GitGuru7
Copy link
Contributor

Do we have intentionally set Default minimum delay to 1 hour in Normal timelock on basemainnet?
https://basescan.org/address/0x21c12f2946a1a66cBFf7eb997022a37167eCf517#code

it("Normal Timelock should be the owner of the Vtreasury", async () => {
expect(await treasury.owner()).equals(basemainnet.NORMAL_TIMELOCK);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also test for pool registry's ownership

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

@web3rover
Copy link
Contributor

web3rover commented Dec 19, 2024

Do we have intentionally set Default minimum delay to 1 hour in Normal timelock on basemainnet? https://basescan.org/address/0x21c12f2946a1a66cBFf7eb997022a37167eCf517#code

@GitGuru7 No. It was the minimum delay by default in the contract https://github.com/VenusProtocol/governance-contracts/blob/develop/contracts/Governance/TimelockV8.sol#L17

Should we change it?

@GitGuru7
Copy link
Contributor

Do we have intentionally set Default minimum delay to 1 hour in Normal timelock on basemainnet? https://basescan.org/address/0x21c12f2946a1a66cBFf7eb997022a37167eCf517#code

@GitGuru7 No. It was the minimum delay by default in the contract https://github.com/VenusProtocol/governance-contracts/blob/develop/contracts/Governance/TimelockV8.sol#L17

Should we change it?

@chechu WDYT? We've set a 2-day delay on other chains. What should we maintain as the minimum delay for NT on Base.
We usually set 1 hour minimum delay on critical and fast-track timelock.

@chechu
Copy link
Contributor Author

chechu commented Dec 20, 2024

Do we have intentionally set Default minimum delay to 1 hour in Normal timelock on basemainnet? https://basescan.org/address/0x21c12f2946a1a66cBFf7eb997022a37167eCf517#code

@GitGuru7 No. It was the minimum delay by default in the contract https://github.com/VenusProtocol/governance-contracts/blob/develop/contracts/Governance/TimelockV8.sol#L17
Should we change it?

@chechu WDYT? We've set a 2-day delay on other chains. What should we maintain as the minimum delay for NT on Base. We usually set 1 hour minimum delay on critical and fast-track timelock.

I would keep it as it is (1 hour), because 1) VIP-408 is already granting this Normal Timelock permissions, and 2) because any change on the delay will be via VIP, so it can be reviewed before execution

Copy link
Contributor Author

@chechu chechu left a comment

Choose a reason for hiding this comment

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

Commands lgtm

@chechu chechu mentioned this pull request Dec 20, 2024
@web3rover
Copy link
Contributor

@chechu
Copy link
Contributor Author

chechu commented Dec 20, 2024

Copy link
Contributor Author

@chechu chechu left a comment

Choose a reason for hiding this comment

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

Mainnet commands lgtm

Copy link
Contributor Author

@chechu chechu left a comment

Choose a reason for hiding this comment

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

The transfer of ownership of the NTG lgtm

@chechu
Copy link
Contributor Author

chechu commented Dec 22, 2024

@web3rover web3rover merged commit 2395e37 into main Dec 23, 2024
2 checks passed
@web3rover web3rover deleted the feat/VEN-2882-ownership-main branch December 23, 2024 11:13
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.

5 participants