Skip to content

Fixes Issue 25: Implement goal-based and rotational group types #29

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.

Sign up for GitHub

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

Merged
Queenode merged 6 commits into from
Jul 21, 2026
Merged

Fixes Issue 25: Implement goal-based and rotational group types #29

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0b720b9
chore: bump soroban-sdk to 27.0.0
Ultra-Tech-code Jul 19, 2026
c85f6a3
feat: implement GoalBased and Rotational group types
Ultra-Tech-code Jul 19, 2026
d165dde
test: add tests for GoalBased group types
Ultra-Tech-code Jul 19, 2026
af23dc3
fix: update test_events assertion to pass in newer SDK
Ultra-Tech-code Jul 19, 2026
a19eeee
chore: track Cargo.lock and test snapshots to fix CI build
Ultra-Tech-code Jul 20, 2026
ba59604
ci: use wasm32v1-none target for Soroban SDK 27.0.0 and Rust 1.84+
Ultra-Tech-code Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions contracts/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ edition = "2021"
crate-type = ["cdylib", "rlib"]

[dependencies]
soroban-sdk = "21.0.0"
soroban-sdk = "27.0.0"

[dev-dependencies]
soroban-sdk = { version = "21.0.0", features = ["testutils"] }
soroban-sdk = { version = "27.0.0", features = ["testutils"] }
Comment on lines +10 to +13

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

soroban-sdk 27.0.0 migration breaking changes from 21

💡 Result:

Migrating from soroban-sdk v21 to v27 involves significant changes, as you are skipping several major releases (v22 through v26) in addition to the changes in v27 [1][2][3]. Key migration areas include: 1. Protocol Support and Environment: - SDK v27 is adapted for Stellar Protocol 27 [4]. - You must update your environment to build for the wasm32v1-none target using stellar-cli (do not use cargo build directly) [5]. - The dependency on stellar-xdr has been updated, and the curr feature flag has been removed [4]. 2. CAP-71 Authorization Delegation: - SDK v27 introduces support for CAP-71, adding new APIs for delegated authentication [4]. - New functions include Address::delegate_account_auth and Env::get_delegated_signers_for_current_auth_check [4]. 3. Major Breaking Changes Across Versions (v21-v27): - v22: Env::register and Env::register_at replaced previous registration methods; DeployerWithAddress::deploy_v2 replaced deploy; fuzz_catch_panic was deprecated [2]. - v23: contractevent replaced Events::publish; MuxedAddress replaced Address in certain TokenInterface methods; changes were made to how archived persistent entries are tested [2]. - v27 Specific: Functional changes were introduced regarding cfg attributes and export arguments (see the _migrating module in the SDK source for detailed guidance) [1][3]. Recommendations: - Consult the _migrating module within the soroban-sdk documentation or source code for a comprehensive, version-by-version summary [5][6][3]. - Review the GitHub release notes for every major version release between v21 and v27 to identify specific breaking changes relevant to your contract code [1][7]. - Update your test suite, as snapshot formats for protocol v27 have been updated and some diagnostic events are no longer captured in snapshots [4][2].

Citations:

🏁 Script executed:

git ls-files | sed -n '1,220p'

Repository: Kolo-Org/Kolo-smartcontract

Length of output: 961

🏁 Script executed:

fd -H -I -t f 'Cargo.toml|rust-toolchain|*.yml|*.yaml|*.rs' .

Repository: Kolo-Org/Kolo-smartcontract

Length of output: 514

🏁 Script executed:

git ls-files | sed -n '1,240p'

Repository: Kolo-Org/Kolo-smartcontract

Length of output: 961

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'FILES\n'
git ls-files | rg '(^|/)(Cargo.toml|rust-toolchain(\.toml)?|\.github/workflows/.*\.(ya?ml)|.*\.rs)$' | sed -n '1,240p'

Repository: Kolo-Org/Kolo-smartcontract

Length of output: 312

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== contracts/Cargo.toml ==\n'
cat -n contracts/Cargo.toml

printf '\n== .github/workflows/rust.yml ==\n'
cat -n .github/workflows/rust.yml

printf '\n== github_actions/workflows/ci.yml ==\n'
cat -n github_actions/workflows/ci.yml

printf '\n== contracts/src/lib.rs ==\n'
cat -n contracts/src/lib.rs

printf '\n== contracts/src/test.rs ==\n'
cat -n contracts/src/test.rs

printf '\n== contracts/deploy.sh ==\n'
cat -n contracts/deploy.sh

Repository: Kolo-Org/Kolo-smartcontract

Length of output: 43935

Update the Soroban 27 compatibility path. .github/workflows/rust.yml and contracts/deploy.sh still build with cargo build --target wasm32-unknown-unknown, but this SDK bump should move to the newer Soroban CLI/target. contracts/src/lib.rs also still uses env.events().publish, so the contract/tests need a compatibility pass before merge.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/Cargo.toml` around lines 10 - 13, Update the Soroban 27
compatibility path across the Rust workflow, contracts/deploy.sh, and the
contract implementation in contracts/src/lib.rs: replace the legacy wasm32 cargo
build flow with the newer Soroban CLI/target commands, and migrate
env.events().publish usage plus related tests to the Soroban 27-compatible API.
Keep deployment and test behavior unchanged while ensuring all build,
deployment, and contract event paths use the updated SDK conventions.


[profile.release]
opt-level = "z"
Expand All @@ -19,3 +19,4 @@ debug = 0
strip = "debuginfo"
codegen-units = 1
panic = "abort"

133 changes: 118 additions & 15 deletions contracts/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ fn extend_instance_ttl(env: &Env) {
.extend_ttl(LEDGERS_TO_LIVE / 2, LEDGERS_TO_LIVE);
}

#[contracttype]
#[derive(Clone, PartialEq, Eq)]
pub enum GroupType {
Rotational,
GoalBased,
}

#[contracttype]
#[derive(Clone)]
pub enum DataKey {
Expand All @@ -26,6 +33,9 @@ pub enum DataKey {
HasContributedThisCycle(Address),
CycleMemberCount,
User(Address),
GroupType,
TargetAmount,
LockUntilTarget,
}

#[contracttype]
Expand All @@ -47,6 +57,9 @@ impl KoloSavingsContract {
token: Address,
name: String,
contribution_amount: i128,
group_type: GroupType,
target_amount: Option<i128>,
lock_until_target: bool,
) {
if env.storage().instance().has(&DataKey::Admin) {
panic!("Already initialized");
Expand All @@ -61,6 +74,13 @@ impl KoloSavingsContract {
env.storage()
.instance()
.set(&DataKey::ContributionAmount, &contribution_amount);
env.storage().instance().set(&DataKey::GroupType, &group_type);
if let Some(target) = target_amount {
env.storage().instance().set(&DataKey::TargetAmount, &target);
}
env.storage()
.instance()
.set(&DataKey::LockUntilTarget, &lock_until_target);
Comment on lines +78 to +83

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate target_amount when lock_until_target is enabled.

When lock_until_target is true but target_amount is None, no TargetAmount key is written. In withdraw_savings, the lock check is wrapped in if let Some(target_amount) = ...get(TargetAmount), so a missing key silently bypasses the lock entirely and permits immediate withdrawal — the opposite of the intended "locked until target" guarantee. Reject this inconsistent configuration at init (and ideally require target > 0).

🛡️ Proposed guard 
 env.storage().instance().set(&DataKey::GroupType, &group_type);
+if lock_until_target && target_amount.is_none() {
+    panic!("lock_until_target requires a target_amount");
+}
 if let Some(target) = target_amount {
+    if target <= 0 {
+        panic!("target_amount must be positive");
+    }
     env.storage().instance().set(&DataKey::TargetAmount, &target);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if let Some(target) = target_amount {
env.storage().instance().set(&DataKey::TargetAmount, &target);
}
env.storage()
.instance()
.set(&DataKey::LockUntilTarget, &lock_until_target);
if lock_until_target && target_amount.is_none() {
panic!("lock_until_target requires a target_amount");
}
if let Some(target) = target_amount {
if target <= 0 {
panic!("target_amount must be positive");
}
env.storage().instance().set(&DataKey::TargetAmount, &target);
}
env.storage()
.instance()
.set(&DataKey::LockUntilTarget, &lock_until_target);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/src/lib.rs` around lines 78 - 83, Validate the configuration in the
initialization logic before persisting values: when lock_until_target is true,
require target_amount to be Some and greater than zero, rejecting otherwise.
Preserve the existing TargetAmount storage behavior for valid configurations and
use the contract’s established error mechanism to abort invalid initialization.


let empty_members: Vec<Address> = Vec::new(&env);
env.storage()
Expand Down Expand Up @@ -109,21 +129,29 @@ impl KoloSavingsContract {
.instance()
.get(&DataKey::ContributionAmount)
.unwrap();
if amount != expected_amount {
let group_type: GroupType = env.storage().instance().get(&DataKey::GroupType).unwrap_or(GroupType::Rotational);

if group_type == GroupType::Rotational && amount != expected_amount {
panic!("Must contribute the exact amount");
}

if amount <= 0 {
panic!("Amount must be positive");
}

let members: Vec<Address> = env.storage().instance().get(&DataKey::Members).unwrap();
if !members.contains(&member) {
panic!("Not a member");
}

// Freeze the member count at the start of a cycle on the first contribution
if !env.storage().instance().has(&DataKey::CycleMemberCount) {
let count = members.len() as i128;
env.storage()
.instance()
.set(&DataKey::CycleMemberCount, &count);
if group_type == GroupType::Rotational {
if !env.storage().instance().has(&DataKey::CycleMemberCount) {
let count = members.len() as i128;
env.storage()
.instance()
.set(&DataKey::CycleMemberCount, &count);
}
}
Comment on lines +132 to 155

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

GoalBased contributions are not actually "flexible" in cadence.

The exact-amount and cycle-freeze branches are correctly gated to Rotational, but the downstream HasContributedThisCycle gate (Lines 157‑164) is still applied to GoalBased groups. A GoalBased member can therefore contribute only once until an admin calls reset_cycle(), which contradicts the goal-based "flexible contributions" objective and is confirmed by the workaround in test_goalbased_flexible_contributions. Consider skipping the once-per-cycle gate for GoalBased (the cycle concept only applies to rotational payouts).

🧰 Tools
🪛 Clippy (1.96.0)

[warning] 148-148: this if statement can be collapsed

(warning)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/src/lib.rs` around lines 132 - 155, Update the downstream
HasContributedThisCycle validation to apply only when group_type is Rotational,
allowing GoalBased members to contribute multiple times without requiring
reset_cycle(). Preserve the existing rotational once-per-cycle restriction and
use the GroupType value established in the contribution flow.


let has_contributed: bool = env
Expand Down Expand Up @@ -173,6 +201,11 @@ impl KoloSavingsContract {
/// Withdraw payout (Admin triggers payout to a member)
/// Enforces strictly fixed rotational payout (Ajo/Esusu) rules.
pub fn payout(env: Env, recipient: Address) {
let group_type: GroupType = env.storage().instance().get(&DataKey::GroupType).unwrap_or(GroupType::Rotational);
if group_type == GroupType::GoalBased {
panic!("Payouts not allowed in GoalBased groups");
}

let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap();
admin.require_auth();
extend_instance_ttl(&env);
Expand Down Expand Up @@ -225,25 +258,93 @@ impl KoloSavingsContract {
.publish((symbol_short!("payout"), recipient), pool_size);
}

/// Withdraw savings (GoalBased groups only)
pub fn withdraw_savings(env: Env, member: Address, amount: i128) {
member.require_auth();
extend_instance_ttl(&env);

let group_type: GroupType = env
.storage()
.instance()
.get(&DataKey::GroupType)
.unwrap_or(GroupType::Rotational);

if group_type == GroupType::Rotational {
panic!("Withdrawals not allowed in rotational groups");
}

if amount <= 0 {
panic!("Withdrawal amount must be positive");
}

let current_contribution: i128 = env
.storage()
.persistent()
.get(&DataKey::Contributions(member.clone()))
.unwrap_or(0);

if current_contribution < amount {
panic!("Insufficient savings to withdraw");
}

let lock_until_target: bool = env
.storage()
.instance()
.get(&DataKey::LockUntilTarget)
.unwrap_or(false);

if lock_until_target {
if let Some(target_amount) = env.storage().instance().get::<_, i128>(&DataKey::TargetAmount) {
if current_contribution < target_amount {
panic!("Target amount not reached yet");
}
}
}
Comment on lines +296 to +302

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Lock re-engages after a partial withdrawal.

The gate compares the live current_contribution against target_amount. Once a member reaches the target and withdraws part of it, the remaining balance drops below target_amount, so any subsequent withdrawal panics with "Target amount not reached yet" — locking funds the member already unlocked. Track a "target reached" flag (persisted once reached), or gate on cumulative contributions rather than the mutable balance.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/src/lib.rs` around lines 296 - 302, Update the lock check around
DataKey::TargetAmount so reaching the target is persisted independently of
mutable current_contribution. Add or reuse a stored target-reached flag, set it
when the contribution first reaches target_amount, and have subsequent
withdrawal gating consult that flag so partial withdrawals do not re-engage the
lock.


let new_contribution = current_contribution - amount;
env.storage().persistent().set(
&DataKey::Contributions(member.clone()),
&new_contribution,
);
env.storage().persistent().extend_ttl(
&DataKey::Contributions(member.clone()),
LEDGERS_TO_LIVE / 2,
LEDGERS_TO_LIVE,
);

let token: Address = env.storage().instance().get(&DataKey::Token).unwrap();
let token_client = token::Client::new(&env, &token);

token_client.transfer(&env.current_contract_address(), &member, &amount);

env.events()
.publish((symbol_short!("withdraw"), member), amount);
Comment on lines +320 to +321

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

soroban-sdk 27 events publish deprecated contractevent migration

💡 Result:

In the soroban-sdk, the Events::publish method has been deprecated in favor of the #[contractevent] macro [1]. This migration was originally introduced in version 23 and has become standard practice [2][3]. To migrate from Events::publish to #[contractevent]: 1. Define a struct for your event and annotate it with #[contractevent]. By default, this uses a single static topic (the event name in snake_case) and treats fields not marked as #[topic] as data collected into a Map [2][4]. 2. Use the #[topic] attribute on fields that should be used as indexed topics [2][4]. 3. If necessary, customize the event's static topics using the topics attribute (e.g., #[contractevent(topics = [...])]) [2][4]. 4. Publish the event by calling .publish(&env) on an instance of your struct [4]. The #[contractevent] macro is preferred because it provides type safety and includes the event definition in the contract interface specification, which allows tooling, SDKs, and generated clients to better understand and interact with published events [2][5]. In newer versions of the SDK (including the v27 series), legacy event handling mechanisms have been systematically removed [6]. If you are working on a codebase transitioning to SDK 27, you should replace all env.events().publish(...) calls with the #[contractevent] macro [3].

Citations:

Migrate the remaining legacy event publishes

env.events().publish(...) is deprecated in soroban-sdk 27. Replace the remaining calls in contracts/src/lib.rs with #[contractevent] structs and Event.publish(&env) to avoid warning-as-error failures.

🧰 Tools
🪛 Clippy (1.96.0)

[warning] 321-321: use of deprecated method soroban_sdk::events::Events::publish

(warning)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/src/lib.rs` around lines 320 - 321, Replace the legacy
env.events().publish call in the contract’s withdrawal flow with a
#[contractevent] event struct representing the withdraw event and publish it via
the generated Event.publish(&env) API. Update all remaining legacy publish calls
in contracts/src/lib.rs consistently, preserving each event’s existing topic
values and payloads.

Source: Linters/SAST tools

}

/// Resets the payout cycle so members can receive payouts again.
pub fn reset_cycle(env: Env) {
let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap();
admin.require_auth();
extend_instance_ttl(&env);

let group_type: GroupType = env.storage().instance().get(&DataKey::GroupType).unwrap_or(GroupType::Rotational);
let members: Vec<Address> = env.storage().instance().get(&DataKey::Members).unwrap();

for member in members.iter() {
env.storage()
.persistent()
.set(&DataKey::HasReceivedPayout(member.clone()), &false);
if group_type == GroupType::Rotational {
env.storage()
.persistent()
.set(&DataKey::HasReceivedPayout(member.clone()), &false);
env.storage().persistent().extend_ttl(
&DataKey::HasReceivedPayout(member.clone()),
LEDGERS_TO_LIVE / 2,
LEDGERS_TO_LIVE,
);
}

env.storage()
.persistent()
.set(&DataKey::HasContributedThisCycle(member.clone()), &false);
env.storage().persistent().extend_ttl(
&DataKey::HasReceivedPayout(member.clone()),
LEDGERS_TO_LIVE / 2,
LEDGERS_TO_LIVE,
);
env.storage().persistent().extend_ttl(
&DataKey::HasContributedThisCycle(member.clone()),
LEDGERS_TO_LIVE / 2,
Expand All @@ -252,7 +353,9 @@ impl KoloSavingsContract {
}

// Clear the frozen member count so it is re-established at the next cycle's first contribution
env.storage().instance().remove(&DataKey::CycleMemberCount);
if group_type == GroupType::Rotational {
env.storage().instance().remove(&DataKey::CycleMemberCount);
}

env.events().publish((symbol_short!("reset"),), ());
}
Expand Down
Loading
Loading