From 74ca1e16f3616e328dc5d2d290cac1a890a69b75 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 13 Nov 2024 00:08:50 +0100 Subject: [PATCH 1/4] feat: improvements --- docs/modules/ROOT/pages/api/finance.adoc | 6 ++++-- packages/finance/src/vesting/vesting.cairo | 23 ++++++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/docs/modules/ROOT/pages/api/finance.adoc b/docs/modules/ROOT/pages/api/finance.adoc index 7d3b5828d..84d142bc5 100755 --- a/docs/modules/ROOT/pages/api/finance.adoc +++ b/docs/modules/ROOT/pages/api/finance.adoc @@ -91,7 +91,7 @@ Returns the total vested amount of a specified `token` at a given `timestamp`. Releases the amount of a given `token` that has already vested and returns that amount. -Emits {AmountReleased} event. +Emits an {AmountReleased} event. [#IVesting-Events] ==== Events @@ -208,11 +208,13 @@ Returns the total vested amount of a specified `token` at a given `timestamp`. Releases the amount of a given `token` that has already vested and returns that amount. +NOTE: If the releasable amount is zero, this function won't emit the event. + Requirements: - `transfer` call to the `token` must return `true` indicating a successful transfer. -Emits {AmountReleased} event. +May emit an {AmountReleased} event. [#VestingComponent-Internal-Functions] ==== Internal functions diff --git a/packages/finance/src/vesting/vesting.cairo b/packages/finance/src/vesting/vesting.cairo index c7169db6a..1c3fb4143 100644 --- a/packages/finance/src/vesting/vesting.cairo +++ b/packages/finance/src/vesting/vesting.cairo @@ -79,6 +79,10 @@ pub mod VestingComponent { ) -> u256; } + // + // External + // + #[embeddable_as(VestingImpl)] impl Vesting< TContractState, @@ -132,21 +136,23 @@ pub mod VestingComponent { /// Releases the amount of a given `token` that has already vested. /// + /// NOTE: If the releasable amount is zero, this function won't emit the event. /// /// Requirements: /// /// - `transfer` call to the `token` must return `true` indicating a successful transfer. /// - /// Emits an `AmountReleased` event. + /// May emit an `AmountReleased` event. fn release(ref self: ComponentState, token: ContractAddress) -> u256 { let amount = self.releasable(token); - self.Vesting_released.write(token, self.Vesting_released.read(token) + amount); - self.emit(AmountReleased { token, amount }); - - let beneficiary = get_dep_component!(@self, Ownable).owner(); - let token_dispatcher = IERC20Dispatcher { contract_address: token }; - assert(token_dispatcher.transfer(beneficiary, amount), Errors::TOKEN_TRANSFER_FAILED); + if amount > 0 { + self.Vesting_released.write(token, self.Vesting_released.read(token) + amount); + self.emit(AmountReleased { token, amount }); + let beneficiary = get_dep_component!(@self, Ownable).owner(); + let token_dispatcher = IERC20Dispatcher { contract_address: token }; + assert(token_dispatcher.transfer(beneficiary, amount), Errors::TOKEN_TRANSFER_FAILED); + } amount } } @@ -159,13 +165,10 @@ pub mod VestingComponent { > of InternalTrait { /// Initializes the component by setting the vesting `start`, `duration` and /// `cliff_duration`. - /// To prevent reinitialization, this should only be used inside of a contract's - /// constructor. /// /// Requirements: /// /// - `cliff_duration` must be less than or equal to `duration`. - /// fn initializer( ref self: ComponentState, start: u64, duration: u64, cliff_duration: u64 ) { From fb1dd0ceb541e23b0dfb8a80703a40b03b475f88 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 13 Nov 2024 00:11:13 +0100 Subject: [PATCH 2/4] feat: add CHANGELOG entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 922ec161f..dff036f0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - SRC9 (Outside Execution) integration to account presets (#1201) +### Changed (Breaking) + +- VestingComponent `release` function won't emit an event or attempt to transfer when the amount is zero (#1209) + ## 0.19.0 (2024-11-08) ### Added From 32630fa95f5a379c55ac9244651ca29695adabd6 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 13 Nov 2024 00:12:17 +0100 Subject: [PATCH 3/4] fix: linter --- packages/finance/src/vesting/vesting.cairo | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/finance/src/vesting/vesting.cairo b/packages/finance/src/vesting/vesting.cairo index 1c3fb4143..baaa92b73 100644 --- a/packages/finance/src/vesting/vesting.cairo +++ b/packages/finance/src/vesting/vesting.cairo @@ -151,7 +151,9 @@ pub mod VestingComponent { let beneficiary = get_dep_component!(@self, Ownable).owner(); let token_dispatcher = IERC20Dispatcher { contract_address: token }; - assert(token_dispatcher.transfer(beneficiary, amount), Errors::TOKEN_TRANSFER_FAILED); + assert( + token_dispatcher.transfer(beneficiary, amount), Errors::TOKEN_TRANSFER_FAILED + ); } amount } From fd7d8be2d55ab540c895e6a22133ffec85b0e3f1 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 13 Nov 2024 10:42:58 +0100 Subject: [PATCH 4/4] feat: apply review updates --- docs/modules/ROOT/pages/api/finance.adoc | 5 +++-- .../finance/src/tests/test_vesting_linear.cairo | 17 +++++++++++++++++ packages/finance/src/vesting/vesting.cairo | 3 ++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/modules/ROOT/pages/api/finance.adoc b/docs/modules/ROOT/pages/api/finance.adoc index 84d142bc5..2f921b6cc 100755 --- a/docs/modules/ROOT/pages/api/finance.adoc +++ b/docs/modules/ROOT/pages/api/finance.adoc @@ -91,7 +91,7 @@ Returns the total vested amount of a specified `token` at a given `timestamp`. Releases the amount of a given `token` that has already vested and returns that amount. -Emits an {AmountReleased} event. +May emit an {AmountReleased} event. [#IVesting-Events] ==== Events @@ -208,7 +208,8 @@ Returns the total vested amount of a specified `token` at a given `timestamp`. Releases the amount of a given `token` that has already vested and returns that amount. -NOTE: If the releasable amount is zero, this function won't emit the event. +NOTE: If the releasable amount is zero, this function won't emit the event +or attempt to transfer the tokens. Requirements: diff --git a/packages/finance/src/tests/test_vesting_linear.cairo b/packages/finance/src/tests/test_vesting_linear.cairo index fc4045c8a..60987ba36 100644 --- a/packages/finance/src/tests/test_vesting_linear.cairo +++ b/packages/finance/src/tests/test_vesting_linear.cairo @@ -6,6 +6,7 @@ use openzeppelin_access::ownable::interface::{IOwnableDispatcher, IOwnableDispat use openzeppelin_test_common::mocks::vesting::LinearVestingMock; use openzeppelin_test_common::vesting::VestingSpyHelpers; use openzeppelin_testing::constants::{OWNER, OTHER}; +use openzeppelin_testing::events::EventSpyExt; use openzeppelin_token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait}; use snforge_std::{spy_events, start_cheat_caller_address, start_cheat_block_timestamp_global}; @@ -103,6 +104,22 @@ fn test_vesting_schedule_with_cliff() { assert_eq!(vesting.vested_amount(token, end_timestamp), data.total_allocation); } +#[test] +fn test_release_zero_amount() { + let data = TEST_DATA(); + let (vesting, token) = setup(data); + + start_cheat_block_timestamp_global(data.start); + let mut spy = spy_events(); + + assert_eq!(vesting.releasable(token), 0); + + let actual_release_amount = vesting.release(token); + assert_eq!(actual_release_amount, 0); + + spy.assert_no_events_left(); +} + #[test] fn test_release_single_call_within_duration() { let data = TEST_DATA(); diff --git a/packages/finance/src/vesting/vesting.cairo b/packages/finance/src/vesting/vesting.cairo index baaa92b73..9aedbc292 100644 --- a/packages/finance/src/vesting/vesting.cairo +++ b/packages/finance/src/vesting/vesting.cairo @@ -136,7 +136,8 @@ pub mod VestingComponent { /// Releases the amount of a given `token` that has already vested. /// - /// NOTE: If the releasable amount is zero, this function won't emit the event. + /// NOTE: If the releasable amount is zero, this function won't emit the event + /// or attempt to transfer the tokens. /// /// Requirements: ///