Skip to content

Conversation

@cwsnt
Copy link
Contributor

@cwsnt cwsnt commented Feb 28, 2024

No description provided.

@cwsnt cwsnt requested a review from tjcloa February 28, 2024 08:48
@tjcloa tjcloa changed the base branch from development to SOV-3686-admin-manager-role March 5, 2024 09:14
@tjcloa tjcloa changed the base branch from SOV-3686-admin-manager-role to development March 5, 2024 09:31

event CSOVReImburse(address from, uint256 CSOVamount, uint256 reImburseAmount);
event CSOVTokensExchanged(address indexed caller, uint256 amount);
contract VestingRegistry is VestingRegistryStorage {
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add natspec - clear description of the purpose of this contract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

);

vestingRegistry = await VestingRegistry.new();
vesting = await VestingRegistryProxy.new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vesting = await VestingRegistryProxy.new();
vestingRegistryProxy = await VestingRegistryProxy.new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

creator // This should be Governance Timelock Contract.
);
vestingRegistry = await VestingRegistry.new();
vesting = await VestingRegistryProxy.new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vesting = await VestingRegistryProxy.new();
vestingRegistryProxy = await VestingRegistryProxy.new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

);

vestingRegistry = await VestingRegistry.new();
vesting = await VestingRegistryProxy.new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vesting = await VestingRegistryProxy.new();
vestingRegistryProxy = await VestingRegistryProxy.new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

// Upgradable Vesting Registry
vestingRegistryLogic = await VestingRegistryLogic.new();
vestingRegistry = await VestingRegistry.new();
vesting = await VestingRegistryProxy.new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vesting = await VestingRegistryProxy.new();
vestingRegistryProxy = await VestingRegistryProxy.new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

// Upgradable Vesting Registry
vestingRegistryLogic = await VestingRegistryLogic.new();
vestingRegistry = await VestingRegistry.new();
vesting = await VestingRegistryProxy.new();
Copy link
Contributor

Choose a reason for hiding this comment

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

pls replace all the occurences in the tests for better readability

Suggested change
vesting = await VestingRegistryProxy.new();
vestingRegistryProxy = await VestingRegistryProxy.new();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

import "../governance/Vesting/VestingRegistry.sol";

contract VestingRegistryLogicMockup is VestingRegistryLogic {
contract VestingRegistryMockup is VestingRegistry {
Copy link
Contributor

Choose a reason for hiding this comment

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

pls also rename the contract file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

});

it("should create vesting contract within vesting period", async () => {
it("should be able to sert investor initial amount list", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like a typo in the test description.

Copy link
Contributor

Choose a reason for hiding this comment

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

also, the test does nothing, pls check the removed tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently, we need to update the OriginInvestorsClaim contract (because initially it was using cliff & duration from staking contract), meanwhile when trying to get the vesting, it's using the cliff & duration from lockedSOV

updated in this commit

investorsClaim.claim({ from: investor1 }),
"OriginInvestorsClaim::onlyWhitelisted: not whitelisted or already claimed"
);
it("investors should be able to claim", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

this test was inverted - why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in this commit

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.

3 participants