From d693552b1813b6c57b011b26160996a9510d4772 Mon Sep 17 00:00:00 2001 From: Maksym <3645723+makarychev@users.noreply.github.com> Date: Mon, 18 Nov 2024 23:08:24 +0200 Subject: [PATCH] [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 --- README.md | 14 ++- app/src/idls/transfer_restrictions.json | 18 +++- app/src/types/transfer_restrictions.ts | 16 ++++ .../src/contexts/initialize_holder_group.rs | 2 +- .../initialize_transfer_restriction_holder.rs | 71 +++++++-------- .../src/contexts/revoke_holder_group.rs | 2 +- programs/transfer-restrictions/src/errors.rs | 4 + .../initialize_holder.rs | 1 + .../initialize_holder_group.rs | 3 + .../transfer_restrictions/revoke_holder.rs | 4 + .../revoke_holder_group.rs | 6 ++ .../initialize-holder.ts | 2 + .../initialize-holdergroup.ts | 9 ++ .../revoke-holder-group.ts | 22 ++++- tests/transfer_restrictions/revoke-holder.ts | 87 +++++++++++++++++++ 15 files changed, 220 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index f6ee4eb..5fd0c59 100644 --- 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 index 25ef411..129f291 100644 --- 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 index 1705e5a..cb7905e 100644 --- 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 index 0ad0775..9eb06e4 100644 --- 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 index c7d7620..ce0a351 100644 --- 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 index f669406..cc345a6 100644 --- 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 index b73c273..b1edda8 100644 --- 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 index ae94ad1..b5993f0 100644 --- 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, 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/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs b/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs index 196f57e..cd6a93f 100644 --- a/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs +++ b/programs/transfer-restrictions/src/instructions/transfer_restrictions/initialize_holder_group.rs @@ -18,5 +18,8 @@ pub fn initialize_holder_group(ctx: Context) -> 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 index 49c8c55..e3f5eb9 100644 --- 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) -> 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 index 66352ca..008d5cb 100644 --- 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) -> 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 index 457df90..cbd2cf1 100644 --- 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 index 094947c..aa72624 100644 --- 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 index 630aa68..e4e89b4 100644 --- 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(); diff --git a/tests/transfer_restrictions/revoke-holder.ts b/tests/transfer_restrictions/revoke-holder.ts index 4d1afb7..cbc779c 100644 --- a/tests/transfer_restrictions/revoke-holder.ts +++ b/tests/transfer_restrictions/revoke-holder.ts @@ -161,6 +161,52 @@ describe("Revoke holder", () => { } }); + it("fails to revoke holder when holder group is not revoked in advance", async () => { + const signer = testEnvironment.transferAdmin; + const [authorityWalletRolePubkey] = + testEnvironment.accessControlHelper.walletRolePDA(signer.publicKey); + const [holderPubkey] = testEnvironment.transferRestrictionsHelper.holderPDA( + new anchor.BN(0) + ); + let accountInfo = await testEnvironment.connection.getAccountInfo( + holderPubkey + ); + assert.isNotNull(accountInfo); + const { currentHoldersCount: currentHoldersCountBefore } = + await testEnvironment.transferRestrictionsHelper.transferRestrictionData(); + + try { + await testEnvironment.transferRestrictionsHelper.program.methods + .revokeHolder() + .accountsStrict({ + holder: holderPubkey, + transferRestrictionData: + testEnvironment.transferRestrictionsHelper + .transferRestrictionDataPubkey, + authorityWalletRole: authorityWalletRolePubkey, + payer: signer.publicKey, + systemProgram: SystemProgram.programId, + }) + .signers([signer]) + .rpc({ commitment: testEnvironment.commitment }); + } catch ({ error }) { + assert.equal(error.errorCode.code, "CurrentHolderGroupCountMustBeZero"); + assert.equal( + error.errorMessage, + "Current holder group count must be zero" + ); + } + + accountInfo = await testEnvironment.connection.getAccountInfo(holderPubkey); + assert.isNotNull(accountInfo); + const { currentHoldersCount: currentHoldersCountAfter } = + await testEnvironment.transferRestrictionsHelper.transferRestrictionData(); + assert.equal( + currentHoldersCountAfter.toNumber(), + currentHoldersCountBefore.toNumber() + ); + }); + it("revokes holder by transfer admin", async () => { const signer = testEnvironment.transferAdmin; const [authorityWalletRolePubkey] = @@ -175,6 +221,27 @@ describe("Revoke holder", () => { const { currentHoldersCount: currentHoldersCountBefore } = await testEnvironment.transferRestrictionsHelper.transferRestrictionData(); + const [holderGroupPubkey] = + testEnvironment.transferRestrictionsHelper.holderGroupPDA( + holderPubkey, + groupId + ); + await testEnvironment.transferRestrictionsHelper.program.methods + .revokeHolderGroup() + .accountsStrict({ + holder: holderPubkey, + holderGroup: holderGroupPubkey, + transferRestrictionData: + testEnvironment.transferRestrictionsHelper + .transferRestrictionDataPubkey, + group: groupPubkey, + authorityWalletRole: authorityWalletRolePubkey, + payer: signer.publicKey, + systemProgram: SystemProgram.programId, + }) + .signers([signer]) + .rpc({ commitment: testEnvironment.commitment }); + await testEnvironment.transferRestrictionsHelper.program.methods .revokeHolder() .accountsStrict({ @@ -212,6 +279,26 @@ describe("Revoke holder", () => { const { currentHoldersCount: currentHoldersCountBefore } = await testEnvironment.transferRestrictionsHelper.transferRestrictionData(); + const [holderGroupPubkey] = + testEnvironment.transferRestrictionsHelper.holderGroupPDA( + holderPubkey, + groupId + ); + await testEnvironment.transferRestrictionsHelper.program.methods + .revokeHolderGroup() + .accountsStrict({ + holder: holderPubkey, + holderGroup: holderGroupPubkey, + transferRestrictionData: + testEnvironment.transferRestrictionsHelper + .transferRestrictionDataPubkey, + group: groupPubkey, + authorityWalletRole: authorityWalletRolePubkey, + payer: signer.publicKey, + systemProgram: SystemProgram.programId, + }) + .signers([signer]) + .rpc({ commitment: testEnvironment.commitment }); await testEnvironment.transferRestrictionsHelper.program.methods .revokeHolder() .accountsStrict({