[HAL-03] HOLDER GROUPS CANNOT BE REVOKED IF THEIR HODLER ARE ALREADY …

…REVOKED (#37) * [HAL-03] HOLDER GROUPS CANNOT BE REVOKED IF THEIR HODLER ARE ALREADY REVOKED * add new errors definition * fix after merge
CoMakery · Nov 18, 2024 · d693552 · d693552
1 parent 0de55fd
commit d693552
Show file tree

Hide file tree

Showing 15 changed files with 220 additions and 41 deletions.
diff --git a/README.md b/README.md
@@ -373,7 +373,9 @@ Typically any legal entity third-party Transfer Agent will need access to both t
 | createHolderFromAddress()  | no             | no            | **yes**        | **yes**       |
 | appendHolderAddress()      | no             | no            | **yes**        | **yes**       |
 | addHolderWithAddresses()   | no             | no            | **yes**        | **yes**       |
-| removeHolder()             | no             | no            | **yes**        | **yes**       |
+| revokeHolder()             | no             | no            | **yes**        | **yes**       |
+| revokeHolderGroup()        | no             | no            | **yes**        | **yes**       |
+| revokeSecurityAssociatedAccount() | no      | no            | **yes**        | **yes**       |
 | createReleaseSchedule()    | **yes**        | **yes**       | **yes**        | **yes**       |
 | mintReleaseSchedule()      | no             | **yes**       | no             | no            |
 
@@ -430,6 +432,16 @@ sequenceDiagram
 5. The Reserve Admin then transfers tokens to the Wallets Admin address.
 6. The Wallets Admin then transfers tokens to Investors or other stakeholders who are entitled to tokens.
 
+## Revoke Holder
+To redeem reserved SOL used for rent-exempt space allocation, you can utilize the `revoke*` methods. It is crucial to revoke accounts in the following sequence:
+
+1. *Revoke Security-Associated Accounts*: Revoke all security-associated accounts for the specified holder.
+2. *Revoke Holder Group Accounts*: Revoke all holder group accounts for the specified holder and associated groups.
+3. *Revoke the Holder*: Finally, revoke the holder account.
+
+A holder can only be revoked if it is not linked to any group or security-associated account. This condition is met when both `current_wallets_count` and `current_holder_group_count` are zero.
+
+
 # Setup For Separate Issuer Private Key Management Roles
 
 By default the reserve tokens cannot be transferred to. To allow transfers the Transfer Admin or Wallets Admin must configure transfer rules using both `updateTransferRestrictionGroup(transferGroup)` to configure the individual account rules and `initialzeTransferRule/updateTransferRule(account, groupFrom, groupTo)` to configure transfers between accounts in a group. A group represents a category like US accredited investors (Reg D) or foreign investors (Reg S).

diff --git a/app/src/idls/transfer_restrictions.json b/app/src/idls/transfer_restrictions.json
@@ -355,7 +355,8 @@
           }
         },
         {
-          "name": "holder"
+          "name": "holder",
+          "writable": true
         },
         {
           "name": "authority_wallet_role"
@@ -1040,6 +1041,7 @@
       "accounts": [
         {
           "name": "holder",
+          "writable": true,
           "pda": {
             "seeds": [
               {
@@ -2105,6 +2107,16 @@
     },
     {
       "code": 6021,
+      "name": "NonPositiveHolderGroupCount",
+      "msg": "Non-positive holder group count"
+    },
+    {
+      "code": 6022,
+      "name": "CurrentHolderGroupCountMustBeZero",
+      "msg": "Current holder group count must be zero"
+    },
+    {
+      "code": 6023,
       "name": "ValueUnchanged",
       "msg": "The provided value is already set. No changes were made"
     }
@@ -2249,6 +2261,10 @@
             "name": "current_wallets_count",
             "type": "u64"
           },
+          {
+            "name": "current_holder_group_count",
+            "type": "u64"
+          },
           {
             "name": "id",
             "type": "u64"

diff --git a/app/src/types/transfer_restrictions.ts b/app/src/types/transfer_restrictions.ts
@@ -296,6 +296,7 @@ export type TransferRestrictions = {
         },
         {
           name: "holder";
+          writable: true;
         },
         {
           name: "authorityWalletRole";
@@ -848,6 +849,7 @@ export type TransferRestrictions = {
       accounts: [
         {
           name: "holder";
+          writable: true;
           pda: {
             seeds: [
               {
@@ -1709,6 +1711,16 @@ export type TransferRestrictions = {
     },
     {
       code: 6021;
+      name: "nonPositiveHolderGroupCount";
+      msg: "Non-positive holder group count";
+    },
+    {
+      code: 6022;
+      name: "currentHolderGroupCountMustBeZero";
+      msg: "Current holder group count must be zero";
+    },
+    {
+      code: 6023;
       name: "valueUnchanged";
       msg: "The provided value is already set. No changes were made";
     }
@@ -1853,6 +1865,10 @@ export type TransferRestrictions = {
             name: "currentWalletsCount";
             type: "u64";
           },
+          {
+            name: "currentHolderGroupCount";
+            type: "u64";
+          },
           {
             name: "id";
             type: "u64";

diff --git a/programs/transfer-restrictions/src/contexts/initialize_holder_group.rs b/programs/transfer-restrictions/src/contexts/initialize_holder_group.rs
@@ -45,7 +45,7 @@ pub struct InitializeHolderGroup<'info> {
     )]
     pub group: Account<'info, TransferRestrictionGroup>,
 
-    #[account(
+    #[account(mut,
       constraint = holder.transfer_restriction_data == transfer_restriction_data.key(),
     )]
     pub holder: Account<'info, TransferRestrictionHolder>,

diff --git a/programs/transfer-restrictions/src/contexts/initialize_transfer_restriction_holder.rs b/programs/transfer-restrictions/src/contexts/initialize_transfer_restriction_holder.rs
@@ -1,48 +1,49 @@
-use anchor_lang::prelude::*;
+use crate::{
+    contexts::common::DISCRIMINATOR_LEN, TransferRestrictionData, TRANSFER_RESTRICTION_DATA_PREFIX,
+};
 use access_control::{self, AccessControl, WalletRole};
-use crate::{contexts::common::DISCRIMINATOR_LEN, TransferRestrictionData, TRANSFER_RESTRICTION_DATA_PREFIX};
+use anchor_lang::prelude::*;
 
 pub const TRANSFER_RESTRICTION_HOLDER_PREFIX: &str = "trh"; // transfer_restriction_holder
 
-
 #[account]
-#[derive(Default)]
-#[derive(InitSpace)]
+#[derive(Default, InitSpace)]
 pub struct TransferRestrictionHolder {
-  pub transfer_restriction_data: Pubkey,
-  pub current_wallets_count: u64,
-  pub id: u64,
-  pub active: bool,
+    pub transfer_restriction_data: Pubkey,
+    pub current_wallets_count: u64,
+    pub current_holder_group_count: u64,
+    pub id: u64,
+    pub active: bool,
 }
 
 #[derive(Accounts)]
 #[instruction(id: u64)]
 pub struct InitializeTransferRestrictionHolder<'info> {
-  #[account(init, payer = payer, space = DISCRIMINATOR_LEN + TransferRestrictionHolder::INIT_SPACE,
-    seeds = [
-      TRANSFER_RESTRICTION_HOLDER_PREFIX.as_bytes(),
-      &transfer_restriction_data.key().to_bytes(),
-      &id.to_le_bytes(),
-    ],
-    bump,
-  )]
-  pub transfer_restriction_holder: Account<'info, TransferRestrictionHolder>,
+    #[account(init, payer = payer, space = DISCRIMINATOR_LEN + TransferRestrictionHolder::INIT_SPACE,
+      seeds = [
+        TRANSFER_RESTRICTION_HOLDER_PREFIX.as_bytes(),
+        &transfer_restriction_data.key().to_bytes(),
+        &id.to_le_bytes(),
+      ],
+      bump,
+    )]
+    pub transfer_restriction_holder: Account<'info, TransferRestrictionHolder>,
 
-  #[account(mut,
-    seeds = [
-      TRANSFER_RESTRICTION_DATA_PREFIX.as_bytes(),
-      &access_control_account.mint.key().to_bytes(),
-    ],
-    bump,
-  )]
-  pub transfer_restriction_data: Account<'info, TransferRestrictionData>,
-  pub access_control_account: Account<'info, AccessControl>,
-  #[account(
-    constraint = authority_wallet_role.owner == payer.key(),
-    constraint = authority_wallet_role.access_control == transfer_restriction_data.access_control_account.key(),
-  )]
-  pub authority_wallet_role: Account<'info, WalletRole>,
-  #[account(mut)]
-  pub payer: Signer<'info>,
-  pub system_program: Program<'info, System>,
+    #[account(mut,
+      seeds = [
+        TRANSFER_RESTRICTION_DATA_PREFIX.as_bytes(),
+        &access_control_account.mint.key().to_bytes(),
+      ],
+      bump,
+    )]
+    pub transfer_restriction_data: Account<'info, TransferRestrictionData>,
+    pub access_control_account: Account<'info, AccessControl>,
+    #[account(
+      constraint = authority_wallet_role.owner == payer.key(),
+      constraint = authority_wallet_role.access_control == transfer_restriction_data.access_control_account.key(),
+    )]
+    pub authority_wallet_role: Account<'info, WalletRole>,
+    #[account(mut)]
+    pub payer: Signer<'info>,
+    pub system_program: Program<'info, System>,
 }
diff --git a/programs/transfer-restrictions/src/contexts/revoke_holder_group.rs b/programs/transfer-restrictions/src/contexts/revoke_holder_group.rs
@@ -6,7 +6,7 @@ use anchor_lang::prelude::*;
 
 #[derive(Accounts)]
 pub struct RevokeHolderGroup<'info> {
-    #[account(
+    #[account(mut,
         seeds = [
             TRANSFER_RESTRICTION_HOLDER_PREFIX.as_bytes(),
             &transfer_restriction_data.key().to_bytes(),

diff --git a/programs/transfer-restrictions/src/errors.rs b/programs/transfer-restrictions/src/errors.rs
@@ -44,6 +44,10 @@ pub enum TransferRestrictionsError {
     NewHolderGroupMaxMustExceedCurrentHolderGroupCount,
     #[msg("Zero group holder group max cannot be non-zero")]
     ZeroGroupHolderGroupMaxCannotBeNonZero,
+    #[msg("Non-positive holder group count")]
+    NonPositiveHolderGroupCount,
+    #[msg("Current holder group count must be zero")]
+    CurrentHolderGroupCountMustBeZero,
     #[msg("The provided value is already set. No changes were made")]
     ValueUnchanged,
 }
diff --git a/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder.rs b/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder.rs
@@ -25,6 +25,7 @@ pub fn initialize_holder(ctx: Context<InitializeTransferRestrictionHolder>, id:
     transfer_restriction_holder.transfer_restriction_data = transfer_restriction_data.key();
     transfer_restriction_holder.id = id;
     transfer_restriction_holder.current_wallets_count = 0;
+    transfer_restriction_holder.current_holder_group_count = 0;
     transfer_restriction_holder.active = true;
     transfer_restriction_data.current_holders_count = transfer_restriction_data
         .current_holders_count

diff --git a/...s/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs b/...s/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs
@@ -18,5 +18,8 @@ pub fn initialize_holder_group(ctx: Context<InitializeHolderGroup>) -> Result<()
     holder_group.holder = ctx.accounts.holder.key();
     holder_group.current_wallets_count = 0;
 
+    let holder = &mut ctx.accounts.holder;
+    holder.current_holder_group_count = holder.current_holder_group_count.checked_add(1).unwrap();
+
     Ok(())
 }
diff --git a/programs/transfer-restrictions/src/instructions/transfer_restrictions/revoke_holder.rs b/programs/transfer-restrictions/src/instructions/transfer_restrictions/revoke_holder.rs
@@ -17,6 +17,10 @@ pub fn revoke_holder(ctx: Context<RevokeHolder>) -> Result<()> {
         holder.current_wallets_count == 0,
         TransferRestrictionsError::CurrentWalletsCountMustBeZero
     );
+    require!(
+        holder.current_holder_group_count == 0,
+        TransferRestrictionsError::CurrentHolderGroupCountMustBeZero
+    );
 
     let transfer_restriction_data = &mut ctx.accounts.transfer_restriction_data;
     transfer_restriction_data.current_holders_count = transfer_restriction_data.current_holders_count.checked_sub(1).unwrap();

diff --git a/programs/transfer-restrictions/src/instructions/transfer_restrictions/revoke_holder_group.rs b/programs/transfer-restrictions/src/instructions/transfer_restrictions/revoke_holder_group.rs
@@ -17,5 +17,11 @@ pub fn revoke_holder_group(ctx: Context<RevokeHolderGroup>) -> Result<()> {
         TransferRestrictionsError::CurrentWalletsCountMustBeZero
     );
 
+    let holder = &mut ctx.accounts.holder;
+    holder.current_holder_group_count = holder
+        .current_holder_group_count
+        .checked_sub(1)
+        .ok_or(TransferRestrictionsError::NonPositiveHolderGroupCount)?;
+
     Ok(())
 }
diff --git a/tests/transfer_restrictions/initialize-holder.ts b/tests/transfer_restrictions/initialize-holder.ts
@@ -147,6 +147,7 @@ describe("Initialize transfer restriction Holder", () => {
     assert.isTrue(holderData.active);
     assert.equal(holderData.id.toString(), zeroHolderIdx.toString());
     assert.equal(holderData.currentWalletsCount.toNumber(), 0);
+    assert.equal(holderData.currentHolderGroupCount.toNumber(), 0);
     assert.equal(
       holderData.transferRestrictionData.toString(),
       testEnvironment.transferRestrictionsHelper.transferRestrictionDataPubkey.toString()
@@ -184,6 +185,7 @@ describe("Initialize transfer restriction Holder", () => {
     assert.isTrue(holderData.active);
     assert.equal(holderData.id.toString(), firstHolderIdx.toString());
     assert.equal(holderData.currentWalletsCount.toNumber(), 0);
+    assert.equal(holderData.currentHolderGroupCount.toNumber(), 0);
     assert.equal(
       holderData.transferRestrictionData.toString(),
       testEnvironment.transferRestrictionsHelper.transferRestrictionDataPubkey.toString()

diff --git a/tests/transfer_restrictions/initialize-holdergroup.ts b/tests/transfer_restrictions/initialize-holdergroup.ts
@@ -183,6 +183,11 @@ describe("Initialize transfer restriction HolderGroup", () => {
     assert.equal(holderGroup.group.toString(), zeroIdx.toString());
     assert.equal(holderGroup.currentWalletsCount.toNumber(), 0);
 
+    const holder = await testEnvironment.transferRestrictionsHelper.holderData(
+      holderPubkey
+    );
+    assert.equal(holder.currentHolderGroupCount.toNumber(), 1);
+
     const { currentHoldersCount: holderGroupCountAfter } =
       await testEnvironment.transferRestrictionsHelper.groupData(groupPubkey);
     assert.equal(
@@ -231,6 +236,10 @@ describe("Initialize transfer restriction HolderGroup", () => {
     assert.equal(holderGroup.holder.toString(), holderPubkey.toString());
     assert.equal(holderGroup.group.toString(), firstGroupIdx.toString());
     assert.equal(holderGroup.currentWalletsCount.toNumber(), 0);
+    const holder = await testEnvironment.transferRestrictionsHelper.holderData(
+      holderPubkey
+    );
+    assert.equal(holder.currentHolderGroupCount.toNumber(), 1);
 
     const { currentHoldersCount: holderGroupCountAfter } =
       await testEnvironment.transferRestrictionsHelper.groupData(groupPubkey);

diff --git a/tests/transfer_restrictions/revoke-holder-group.ts b/tests/transfer_restrictions/revoke-holder-group.ts
@@ -175,8 +175,8 @@ describe("Revoke holder group", () => {
     }
   });
 
-  it("revokes holder group by transfer admin", async () => {
-    const signer = testEnvironment.transferAdmin;
+  it("revokes holder group by wallets admin", async () => {
+    const signer = testEnvironment.walletsAdmin;
     const [authorityWalletRolePubkey] =
       testEnvironment.accessControlHelper.walletRolePDA(signer.publicKey);
     const [holderPubkey] = testEnvironment.transferRestrictionsHelper.holderPDA(
@@ -192,6 +192,8 @@ describe("Revoke holder group", () => {
     );
     assert.isNotNull(accountInfo);
 
+    const { currentHolderGroupCount: currentHolderGroupCountBefore } =
+      await testEnvironment.transferRestrictionsHelper.holderData(holderPubkey);
     await testEnvironment.transferRestrictionsHelper.program.methods
       .revokeHolderGroup()
       .accountsStrict({
@@ -212,6 +214,13 @@ describe("Revoke holder group", () => {
       holderGroupPubkey
     );
     assert.isNull(accountInfo);
+
+    const { currentHolderGroupCount } =
+      await testEnvironment.transferRestrictionsHelper.holderData(holderPubkey);
+    assert.equal(
+      currentHolderGroupCount.toNumber(),
+      currentHolderGroupCountBefore.subn(1).toNumber()
+    );
   });
 
   it("revokes holder group by transfer admin", async () => {
@@ -231,6 +240,8 @@ describe("Revoke holder group", () => {
     );
     assert.isNotNull(accountInfo);
 
+    const { currentHolderGroupCount: currentHolderGroupCountBefore } =
+      await testEnvironment.transferRestrictionsHelper.holderData(holderPubkey);
     await testEnvironment.transferRestrictionsHelper.program.methods
       .revokeHolderGroup()
       .accountsStrict({
@@ -251,6 +262,13 @@ describe("Revoke holder group", () => {
       holderGroupPubkey
     );
     assert.isNull(accountInfo);
+
+    const { currentHolderGroupCount } =
+      await testEnvironment.transferRestrictionsHelper.holderData(holderPubkey);
+    assert.equal(
+      currentHolderGroupCount.toNumber(),
+      currentHolderGroupCountBefore.subn(1).toNumber()
+    );
   });
 
   const investor = Keypair.generate();