Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add assertion to check existence of shareholder in the contract #27

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

sjoshisupra
Copy link

Add assertion to check that a shareholder exist in the contract as a precondition to vest_individual to avoid failure in simple_map::borrow.

Copy link

@nizam-supraoracles nizam-supraoracles left a comment

Choose a reason for hiding this comment

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

I suggest we add the assert_shareholder_exists condition inside the remove_shareholder function too.

@sjoshisupra
Copy link
Author

I suggest we add the assert_shareholder_exists condition inside the remove_shareholder function too.

addressed in the new commit

Copy link

@axiongsupra axiongsupra left a comment

Choose a reason for hiding this comment

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

Thanks for add the check

@@ -383,17 +383,19 @@ module supra_framework::vesting_without_staking {
}

public entry fun vest_individual(contract_address: address, shareholder_address: address) acquires VestingContract {
assert_active_vesting_contract(contract_address);

Choose a reason for hiding this comment

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

@sjoshisupra why we removed this check as the vesting contract can be terminated at the end of vest_individual. It will cause a problem when call vest method who will call vest_individual in a loop.

move termination logic to `vest` instead of `vest_individual`. This is done
to prevent following from happening:
If 5 shareholders are there and only 2 have non-zero balance when `vest` is called
after these 2 get their vesting due, if `vest_individual` sets the termination flag,
in the loop when `vest_individual` is called for 3rd shareholder assertion will fail
and the whole Tx will abort.
@sjoshisupra sjoshisupra merged commit c2f4561 into integrate_consensus_key Jul 16, 2024
1 check passed
@sjoshisupra sjoshisupra deleted the vesting_checks branch July 16, 2024 08:24
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