diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c371ac37..931c71233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed (Breaking) +- VestingComponent `release` function won't emit an event or attempt to transfer when the amount is zero (#1209) - Bump snforge_std to v0.33.0 (#1203) ## 0.19.0 (2024-11-08) diff --git a/docs/modules/ROOT/pages/api/finance.adoc b/docs/modules/ROOT/pages/api/finance.adoc index 7d3b5828d..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 {AmountReleased} event. +May emit an {AmountReleased} event. [#IVesting-Events] ==== Events @@ -208,11 +208,14 @@ 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 +or attempt to transfer the tokens. + 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/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 c7169db6a..9aedbc292 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,26 @@ 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 + /// or attempt to transfer the tokens. /// /// 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 +168,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 ) {