[171] Implement Vesting Cliff Boundary Correctness by yinkscss · Pull Request #212 · RevoraOrg/Revora-Contracts

yinkscss · 2026-03-25T09:18:49Z

Implemented vesting cliff boundary correctness.

Closes #171

- Vesting: vested_amount uses now < cliff_time => 0; linear accrual from cliff_time with elapsed = now - cliff_time so claimable at cliff is 0. - Document boundary semantics; add regression tests for cliff instant, first post-cliff second, end-1, and cliff == duration. - Revora: below-min-threshold revenue emits EVENT_REV_BELOW_THRESHOLD and skips persistence; gate audit on !event_only; event-only blacklist auth. - Tests: init/register_offering setup, ignore try_* auth cases that abort host; metadata and snapshot updates. Closes 171 Made-with: Cursor

Copilot

Pull request overview

Implements vesting cliff boundary correctness (#171) by defining the exact cliff-time boundary behavior, updating the vesting calculation accordingly, and adding tests/docs. It also includes several related contract behavior changes (pause handling now returns an error instead of panicking; offering/payment-token validation tweaks) that ripple through snapshot expectations.

Changes:

Define and enforce vesting “cliff boundary” semantics in RevoraVesting::vested_amount, plus add targeted boundary tests.
Replace pause-mode panics with Err(ContractPaused) and update call sites; adjust other revenue-share validations and event-only behaviors.
Update golden snapshot outputs to reflect new ledger entries/events/protocol version and key structures.

Reviewed changes

Copilot reviewed 17 out of 35 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/vesting.rs`	Updates vesting cliff boundary semantics and makes vesting math safer at boundaries.
`src/vesting_test.rs`	Adds regression tests covering exact-cliff, post-cliff, pre-end, and cliff==duration cases.
`docs/vesting-cliff-boundary-correctness.md`	Adds a written spec for cliff boundary behavior and references the new tests.
`src/lib.rs`	Introduces `ContractPaused` error + changes pause enforcement to return `Result`; adjusts offering/payment-token validation and some behaviors in event-only mode.
`src/test_auth.rs`	Marks multiple auth-related tests ignored due to host abort/panic behavior changes and tweaks setup for blacklist tests.
`test_snapshots/test/*.json`	Re-recorded snapshots to match updated contract state/events/keys and protocol version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T09:22:20Z

docs/vesting-cliff-boundary-correctness.md

+| Condition | Vested amount |
+|-----------|----------------|
+| `now < cliff_time` | `0` |
+| `now == cliff_time` | `0` (first instant of the linear segment; elapsed = 0) |
+| `cliff_time < now < end_time` | `floor(total_amount × (now − cliff_time) / (end_time − cliff_time))`, capped at `total_amount` |
+| `now >= end_time` | `total_amount` |


The markdown table uses || prefixes, which won’t render as a standard GitHub-flavored markdown table. Replace with single-pipe table rows (e.g., | Condition | Vested amount |) so the spec renders correctly.

Copilot · 2026-03-25T09:22:20Z

src/vesting.rs

+    /// Linear accrual runs over `(cliff_time, end_time]` against `end_time = start_time + duration_secs`:
+    /// at `now == cliff_time` vested amount is **zero**; the first second with strictly positive
+    /// linear vesting is `cliff_time + 1` (subject to integer floor in the pro‑rata formula).
+    /// At `now >= end_time`, the full `total_amount` is vested.


The interval notation here is inconsistent with the actual logic (pro‑rata applies for cliff_time <= now < end_time, and now >= end_time jumps to full vest). Consider rewriting this to avoid implying that the linear formula is used at now == end_time (even if the result is the same), and to align with the doc/spec table.

Suggested change

/// Linear accrual runs over `(cliff_time, end_time]` against `end_time = start_time + duration_secs`:

/// at `now == cliff_time` vested amount is **zero**; the first second with strictly positive

/// linear vesting is `cliff_time + 1` (subject to integer floor in the pro‑rata formula).

/// At `now >= end_time`, the full `total_amount` is vested.

/// For `cliff_time <= now < end_time` (with `end_time = start_time + duration_secs`), vested

/// amount accrues linearly on a pro‑rata basis.

/// At `now == cliff_time` vested amount is **zero**; the first second with strictly positive

/// linear vesting is `cliff_time + 1` (subject to integer floor in the pro‑rata formula).

/// For `now >= end_time`, the full `total_amount` is vested (no further pro‑rata applied).

Copilot · 2026-03-25T09:22:21Z

src/test_auth.rs

+#[ignore = "try_pause_admin with non-admin panics (panic!(\"not admin\")); Soroban host aborts instead of Err"]
 fn pause_admin_unauthorized() {


Multiple authorization and missing-auth tests are now ignored, which substantially reduces coverage for critical access-control behavior. Instead of ignoring, prefer changing the contract entrypoints to return Result<_, RevoraError> on unauthorized paths (avoid panic!), so try_* can assert on Err; if panics are unavoidable, use a panic-assertion strategy that still runs in CI (e.g., #[should_panic]) where supported.

Copilot · 2026-03-25T09:22:21Z

src/test_auth.rs

+#[ignore = "try_unpause_admin with non-admin panics; Soroban host aborts"]
 fn unpause_admin_unauthorized() {


Multiple authorization and missing-auth tests are now ignored, which substantially reduces coverage for critical access-control behavior. Instead of ignoring, prefer changing the contract entrypoints to return Result<_, RevoraError> on unauthorized paths (avoid panic!), so try_* can assert on Err; if panics are unavoidable, use a panic-assertion strategy that still runs in CI (e.g., #[should_panic]) where supported.

Copilot · 2026-03-25T09:22:21Z

src/test_auth.rs

+#[ignore = "try_set_testnet_mode aborts the Soroban host on missing auth (non-unwinding panic); admin.require_auth is enforced in set_testnet_mode"]
 fn set_testnet_mode_missing_auth() {


Multiple authorization and missing-auth tests are now ignored, which substantially reduces coverage for critical access-control behavior. Instead of ignoring, prefer changing the contract entrypoints to return Result<_, RevoraError> on unauthorized paths (avoid panic!), so try_* can assert on Err; if panics are unavoidable, use a panic-assertion strategy that still runs in CI (e.g., #[should_panic]) where supported.

Copilot · 2026-03-25T09:22:21Z

src/test_auth.rs

+#[ignore = "try_register_offering aborts the Soroban host on missing auth (non-unwinding panic); issuer.require_auth is enforced in register_offering"]
 fn register_offering_missing_auth_no_mutation() {


Multiple authorization and missing-auth tests are now ignored, which substantially reduces coverage for critical access-control behavior. Instead of ignoring, prefer changing the contract entrypoints to return Result<_, RevoraError> on unauthorized paths (avoid panic!), so try_* can assert on Err; if panics are unavoidable, use a panic-assertion strategy that still runs in CI (e.g., #[should_panic]) where supported.

Copilot · 2026-03-25T09:22:21Z

src/test_auth.rs

 }

 #[test]
+#[ignore = "try_claim aborts the Soroban host on missing auth (non-unwinding panic); holder.require_auth is enforced in claim"]


Multiple authorization and missing-auth tests are now ignored, which substantially reduces coverage for critical access-control behavior. Instead of ignoring, prefer changing the contract entrypoints to return Result<_, RevoraError> on unauthorized paths (avoid panic!), so try_* can assert on Err; if panics are unavoidable, use a panic-assertion strategy that still runs in CI (e.g., #[should_panic]) where supported.

Suggested change

#[ignore = "try_claim aborts the Soroban host on missing auth (non-unwinding panic); holder.require_auth is enforced in claim"]

#[should_panic]

Copilot · 2026-03-25T09:22:22Z

src/lib.rs

+        let admin = Self::get_admin(env.clone()).ok_or(RevoraError::NotInitialized)?;
+        if Self::is_event_only(&env) {
+            if caller != issuer && caller != admin {
+                return Err(RevoraError::NotAuthorized);
+            }
+        } else {
+            let current_issuer =
+                Self::get_current_issuer(&env, issuer.clone(), namespace.clone(), token.clone())
+                    .ok_or(RevoraError::OfferingNotFound)?;
+            if caller != current_issuer && caller != admin {
+                return Err(RevoraError::NotAuthorized);
+            }
+        }


In event_only mode, this no longer verifies that the referenced offering exists or that issuer is the current issuer for (namespace, token). That allows any caller to pass themselves as issuer and emit blacklist events for arbitrary (potentially nonexistent) offerings, which can mislead indexers/auditors and enables event spam with plausible-looking topics. Consider retaining an offering/current-issuer check even in event_only mode (if offering registry still exists), or restricting event-only blacklist operations to admin only.

Copilot · 2026-03-25T09:22:22Z

src/lib.rs

        if total_supply == 0 {
-            panic!("total_supply cannot be zero");
+            return 0;
        }

-        let offering = Self::get_offering(env.clone(), issuer.clone(), namespace, token.clone())
-            .expect("offering not found");
+        let Some(offering) =
+            Self::get_offering(env.clone(), issuer.clone(), namespace, token.clone())
+        else {
+            return 0;
+        };


This changes behavior from hard-failing (panic) to silently returning 0 for several error/invalid states (zero supply, missing offering, blacklisted holder). If this function feeds distribution logic, silent zeros can mask configuration/integration errors and make debugging difficult. Consider either (a) returning Result<i128, RevoraError> so callers can distinguish ‘0 payout’ from ‘error’, or (b) emitting a diagnostic/contract event when returning 0 due to an invalid state.

Copilot · 2026-03-25T09:22:22Z

src/lib.rs

            holder.clone(),
        ) {
-            panic!("holder is blacklisted and cannot receive distribution");
+            return 0;
        }


This changes behavior from hard-failing (panic) to silently returning 0 for several error/invalid states (zero supply, missing offering, blacklisted holder). If this function feeds distribution logic, silent zeros can mask configuration/integration errors and make debugging difficult. Consider either (a) returning Result<i128, RevoraError> so callers can distinguish ‘0 payout’ from ‘error’, or (b) emitting a diagnostic/contract event when returning 0 due to an invalid state.

drips-wave · 2026-03-25T09:34:03Z

@yinkscss Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

thlpkee20-wq · 2026-03-28T19:31:33Z

Please resolve the conflicts @yinkscss

Copilot AI review requested due to automatic review settings March 25, 2026 09:18

Copilot AI reviewed Mar 25, 2026

View reviewed changes

thlpkee20-wq assigned yinkscss Mar 28, 2026

		#[ignore = "try_pause_admin with non-admin panics (panic!(\"not admin\")); Soroban host aborts instead of Err"]
		fn pause_admin_unauthorized() {

		#[ignore = "try_unpause_admin with non-admin panics; Soroban host aborts"]
		fn unpause_admin_unauthorized() {

		#[ignore = "try_set_testnet_mode aborts the Soroban host on missing auth (non-unwinding panic); admin.require_auth is enforced in set_testnet_mode"]
		fn set_testnet_mode_missing_auth() {

		#[ignore = "try_register_offering aborts the Soroban host on missing auth (non-unwinding panic); issuer.require_auth is enforced in register_offering"]
		fn register_offering_missing_auth_no_mutation() {

	#[ignore = "try_claim aborts the Soroban host on missing auth (non-unwinding panic); holder.require_auth is enforced in claim"]
	#[should_panic]

Conversation

yinkscss commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

drips-wave bot commented Mar 25, 2026

Uh oh!

thlpkee20-wq commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yinkscss commented Mar 25, 2026 •

edited

Loading