Skip to content

feat(pool): fix #66 by adding admin add_member and remove_member with database synchronization#76

Merged
Sendi0011 merged 3 commits into
JointSave-org:mainfrom
Dopezapha:feat/pool-member-management
Jun 21, 2026
Merged

feat(pool): fix #66 by adding admin add_member and remove_member with database synchronization#76
Sendi0011 merged 3 commits into
JointSave-org:mainfrom
Dopezapha:feat/pool-member-management

Conversation

@Dopezapha

Copy link
Copy Markdown
Contributor

Closes #66

PR Description

This PR resolves #66 by implementing add_member and remove_member across the Rotational, Target, and Flexible pool smart contracts and integrating these admin actions into the frontend GroupActions interface.

Currently, membership is locked at pool deployment. This change allows pool administrators to manage the member list after creation, enabling them to eject inactive users or add latecomers before rounds start (for Rotational) or the pool unlocks (for Target). When removing members from Flexible or Target pools, the smart contract automatically refunds their active balances and updates the contract's total balance. For Rotational pools, the payout indices and round state are shifted when members are removed to ensure the pool completes correctly when the final beneficiary is removed.

Additionally, this PR syncs the Supabase pool_members table on PATCH requests so that frontend views (like GroupMembers and page refreshes) always match the live ledger.

Changes Made

  • Added add_member and remove_member entrypoints with admin authorization checks in all three pool contracts.
    • smartcontract/contracts/rotational/src/lib.rs
    • smartcontract/contracts/target/src/lib.rs
    • smartcontract/contracts/flexible/src/lib.rs
  • Handled balance refunds on member removal in target and flexible pools.
  • Added logic to shift beneficiary indexes and complete/inactivate the pool if the last active beneficiary is removed in rotational pools.
  • Added frontend components and transaction handlers under GroupActions for pool admins to manage members, including Stellar address validation and confirmation modals.
    • frontend/components/group/group-actions.tsx
    • frontend/hooks/useJointSaveContracts.ts
  • Resolved strict TypeScript type-checking regressions in group-details.tsx and yield-dashboard.tsx.
  • Updated /api/pools PATCH endpoint in frontend/app/api/pools/route.ts to insert and delete member entries in the database to align with on-chain membership edits.
  • Added Rust unit tests covering both positive and negative membership update scenarios.
    • smartcontract/contracts/*/src/tests.rs

@Sendi0011 Sendi0011 self-requested a review June 20, 2026 19:31

@Sendi0011 Sendi0011 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Dopezapha i Read through all three contracts directly, especially the Rotational round-shifting logic since that's the trickiest part here — traced through removing a member before/at the current round and down to the last remaining member, and the math holds up correctly in all three cases I checked by hand. Good work on that.
Three things before merging:

1: There's no remove_member test for the Rotational pool specifically (Target and Flexible both have one). Given how subtle the round-index math is, this is the one contract that really needs a test to lock in correctness against future changes.

2: remove_member doesn't check the Paused flag, unlike deposit/withdraw/distribute_yield — worth a deliberate call on whether member removal (which moves real funds via refund) should be blocked while a pool is paused.

3: In frontend/app/api/pools/route.ts, the new sync code does body.memberAddress.toLowerCase() on insert and delete, but nothing else in that file lowercases addresses. Stellar addresses are uppercase by convention — can you confirm how addresses are stored when a pool is first created? If they're stored as-is (uppercase), this delete query would silently match nothing when removing a member.

@Dopezapha

Copy link
Copy Markdown
Contributor Author

Kindly review

@Dopezapha Dopezapha force-pushed the feat/pool-member-management branch from e98d9e0 to ca73576 Compare June 20, 2026 22:27
@Sendi0011 Sendi0011 requested review from Sendi0011 June 21, 2026 20:34

@Sendi0011 Sendi0011 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Went through the updated commit in detail — all three things from my earlier review are properly addressed.
The two new tests (test_remove_member_general and test_remove_last_beneficiary_completes_pool) genuinely cover the boundary cases that matter, including the trickiest one — removing the current beneficiary down to pool completion. Verified the pause check is now correctly in place on remove_member too, with its own test.
One correction on my end: I'd flagged the .toLowerCase() on memberAddress as inconsistent with the rest of the file, but looking more carefully, creatorAddress and user_address were already being lowercased elsewhere in this same file before this PR — so it's actually consistent with the existing convention, not a new issue. My mistake for not checking further up the file initially.
Re-ran the typecheck on this branch with main merged in (post-#78) and it's clean.

@Sendi0011

Copy link
Copy Markdown
Contributor

Approving, nice work tightening this up.

@Sendi0011 Sendi0011 merged commit 2ceb596 into JointSave-org:main Jun 21, 2026
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.

[Feature] Add add_member and remove_member functions to pool contracts

2 participants