feature(SDK): add DualGovernance module #229
Conversation
phonktown
force-pushed
the
feature/dual-governance-module
branch
from
6bbeb40 to
c2a3a9d
Compare
packages/sdk/src/dual-governance/__tests__/dual-governance.test.ts
Outdated
Show resolved
Hide resolved
phonktown
force-pushed
the
feature/dual-governance-module
branch
2 times, most recently
from
4177504 to
2138e77
Compare
phonktown
force-pushed
the
feature/dual-governance-module
branch
from
2138e77 to
f7a3c25
Compare
packages/sdk/src/common/constants.ts
Outdated
| @@ -92,6 +92,14 @@ export enum LIDO_CONTRACT_NAMES { | |||
| wsteth = 'wsteth', | |||
| } | |||
|
|
|||
| export const EMERGENCY_PROTECTED_TIMELOCK_ADDRESSES: { | |||
There was a problem hiding this comment.
Will these addresses be added to Locator?
| const stETHContract = await dualGovernance.getContractStETH(); | ||
| expectContract(stETHContract); | ||
|
|
||
| const stETHAddress = await dualGovernance.getStETHAddress(); |
There was a problem hiding this comment.
It seems that this test is not very useful. It checks the address from the module against the address from the module.
Wouldn't it be better to check the address from the module against the address from the locator?
| expect(result).toBeDefined(); | ||
|
|
||
| // Expect above doesn't solve 'possibly undefined' linter warning, so we still have a condition as below | ||
| if (result !== undefined) { |
There was a problem hiding this comment.
in which case will it be undefined?
| export class LidoSDKDualGovernance extends LidoSDKModule { | ||
| // ---- Contract Addresses ---- | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) |
There was a problem hiding this comment.
looks like here we need a cache for getContractEmergencyProtectedTimelock too
| // ---- Contract Addresses ---- | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) | ||
| public async getGovernanceAddress(): Promise<Address | undefined> { |
There was a problem hiding this comment.
lets use getContractAddress from core module
There was a problem hiding this comment.
Working with Dual Governance we follow a specific approach regarding getting the contract addresses.
The only constant address in this feature is EmergencyProtectedTimelock.
The Governance address is dynamic. So we never access it directly, as well as all the addresses we get from the Governance.
Hence, in the module, as well as in the tests and UI, we only keep EmergencyProtectedTimelock as the source of truth and get the rest contracts through it.
There was a problem hiding this comment.
what is important here is that all contract addresses are present in the Core module.
| } | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) | ||
| public async getVetoSignallingEscrowAddress(): Promise<Address | undefined> { |
| } | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) | ||
| public async getStETHAddress(): Promise<Address | undefined> { |
There was a problem hiding this comment.
already has in the core module
| } | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) | ||
| public async getDualGovernanceConfigProviderAddress(): Promise< |
| GetContractReturnType<typeof emergencyProtectedTimelockAbi, PublicClient> | ||
| > { | ||
| const address = | ||
| EMERGENCY_PROTECTED_TIMELOCK_ADDRESSES[this.core.chain.id as CHAINS]; |
There was a problem hiding this comment.
move to getContractAddress
| }); | ||
| } | ||
|
|
||
| @Cache(30 * 60 * 1000, ['core.chain.id']) |
There was a problem hiding this comment.
pls check cache for all methods
| } | ||
| } | ||
|
|
||
| public async getTotalStEthInEscrow(): Promise<bigint | undefined> { |
There was a problem hiding this comment.
@logger('Views:')
@errorhandler()
And you dont need additional try/catch here
| } | ||
| } | ||
|
|
||
| public async getDualGovernanceConfig(): Promise< |
There was a problem hiding this comment.
@logger('Views:')
@errorhandler()
| } | ||
| } | ||
|
|
||
| public async getTotalStETHSupply(): Promise<bigint | undefined> { |
There was a problem hiding this comment.
@logger('Views:')
@errorhandler()
| } | ||
| } | ||
|
|
||
| public async calculateCurrentVetoSignallingThresholdProgress(): Promise< |
There was a problem hiding this comment.
@logger('Views:')
@errorhandler()
| } | ||
| | undefined | ||
| > { | ||
| const totalStETHSupply = await this.getTotalStETHSupply(); |
| } | ||
| } | ||
|
|
||
| public async getGovernanceWarningStatus({ |
There was a problem hiding this comment.
@logger('Views:')
@errorhandler()
phonktown
dismissed stale reviews from katamarinaki and swissarmytowel
via
027346b
Description
This PR includes:
The latter is to fetch current Governance status in terms of 3 main states [Normal, Warning, Blocked]
The method
getGovernanceWarningStatusis a helper for applications to decide whether any actions should be performed in UI based on the Governance state.Return type of this helper is defined in
types.tsMost of the methods are public for future usage and integrations.
Note
Docs will be added later to cover all the logic the DG module is based on, including test scenarios.
Checklist: