diff --git a/.changeset/fix-batch-transfer-compliance-checks.md b/.changeset/fix-batch-transfer-compliance-checks.md new file mode 100644 index 0000000000..6afc93b8b9 --- /dev/null +++ b/.changeset/fix-batch-transfer-compliance-checks.md @@ -0,0 +1,9 @@ +--- +"@hashgraph/asset-tokenization-contracts": patch +--- + +Fix: `batchTransfer` now validates sender and each destination against the control list and compliance module independently. + +Previously, `batchTransfer` used `onlyCompliant(sender, address(0), false)` for the sender and called `checkCompliance(address(0), to, false)` per destination, passing `address(0)` as the counterpart and `0` as the value in both cases. The sender was never validated as an individual account (only its compliance-contract approval was checked), and each per-destination compliance call omitted the actual sender and amount. + +The sender is now checked via `onlyAccountCompliant(sender)`, which validates recovery status and control-list membership before any token movement. Inside the loop, each destination is checked with `checkAccountCompliance(to)` (recovery + control list) followed by `checkTransferCompliance(sender, to, amounts[i])`, which calls the compliance module with the real sender address and transfer amount. diff --git a/packages/ats/contracts/contracts/domain/asset/ERC1594StorageWrapper.sol b/packages/ats/contracts/contracts/domain/asset/ERC1594StorageWrapper.sol index d43ef80ecc..cd01f9859f 100644 --- a/packages/ats/contracts/contracts/domain/asset/ERC1594StorageWrapper.sol +++ b/packages/ats/contracts/contracts/domain/asset/ERC1594StorageWrapper.sol @@ -42,6 +42,7 @@ struct ERC1594Storage { /** * @title ERC1594StorageWrapper + * @author Asset Tokenization Studio Team * @notice Library providing the core issuance, redemption, and compliance * checking logic for the ERC1594 token standard. Handles storage management, * balance mutations via ERC20/ERC1410 wrappers, and multi-layered access @@ -156,20 +157,6 @@ library ERC1594StorageWrapper { checkIdentity(from, to); } - /** - * @notice Reverts if the transfer between `from` and `to` fails - * compliance checks. Optionally checks sender compliance. - * @dev Forwards to `checkCompliance`. Used when the sender is not - * necessarily the `from` parameter. - * @param from Source address. - * @param to Destination address. - * @param checkSender Whether to perform compliance validation on the - * current `msg.sender`. - */ - function requireCompliant(address from, address to, bool checkSender) internal view { - checkCompliance(from, to, checkSender); - } - /** * @notice Reverts if either `from` or `to` is listed as a recovered * wallet. @@ -313,16 +300,57 @@ library ERC1594StorageWrapper { * @param checkSender Whether to validate compliance of `msg.sender`. */ function checkCompliance(address from, address to, bool checkSender) internal view { + checkCompliance(from, to, 0, checkSender); + } + + /** + * @notice Reverts if the transfer between `from` and `to` fails + * compliance checks. Optionally checks sender compliance. + * @dev Delegates to `_isCompliant` and reverts with the encoded reason + * code and details on failure. + * @param from Source address (may be zero). + * @param to Destination address (may be zero). + * @param value Amount tried to be transferred. + * @param checkSender Whether to validate compliance of `msg.sender`. + */ + function checkCompliance(address from, address to, uint256 value, bool checkSender) internal view { (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _isCompliant( from, to, - 0, + value, EvmAccessors.getMsgSender(), checkSender ); if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details); } + /** + * @notice Reverts if `account` fails individual account compliance checks. + * @dev Delegates to `_isAccountCompliant`, which validates the account against + * the recovery registry and the control list. Zero address is always allowed. + * @param account Address to validate. + */ + function checkAccountCompliance(address account) internal view { + (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _isAccountCompliant(account); + if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details); + } + + /** + * @notice Reverts if the compliance module disallows a transfer from `from` + * to `to` for `value` tokens. + * @dev Delegates to `_validateTransferCompliance`, which calls the compliance + * contract's `canTransfer(from, to, value)` via staticcall. Use this when only + * the compliance module check is required, bypassing recovery and control list + * validation. + * @param from Source address. + * @param to Destination address. + * @param value Amount of tokens to transfer. + */ + function checkTransferCompliance(address from, address to, uint256 value) internal view { + (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _validateTransferCompliance(from, to, value); + if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details); + } + /** * @notice Returns the ERC1594 storage slot using the predefined * position constant. @@ -376,12 +404,57 @@ library ERC1594StorageWrapper { ); } + /** + * @notice Validates compliance for the sender involved in a transfer + * or redemption. + * @dev Checks the sender against recovery, control list, and compliance contract. + * @param from Source address (may be zero). + * @param to Destination address (may be zero). + * @param value Transfer amount. + * @param sender Address of the operator initiating the operation. + * @return status True if all compliance checks pass. + * @return statusCode EIP1066 status byte. + * @return reasonCode Selector of the blocking error. + * @return details Encoded error data (empty on success). + */ + function _isSenderCompliant( + address from, + address to, + uint256 value, + address sender + ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) { + (status, statusCode, reasonCode, details) = _validateAccountForTransfer(sender, abi.encode(sender)); + if (!status) return (status, statusCode, reasonCode, details); + (status, statusCode, reasonCode, details) = _validateSenderCompliance(sender, from, to, value); + if (!status) return (status, statusCode, reasonCode, details); + } + + /** + * @notice Validates compliance for the 'from' involved in a transfer + * or redemption. + * @dev Checks the from against recovery, control list, and compliance contract. + * @param account Account address (may be zero). + * @return status True if all compliance checks pass. + * @return statusCode EIP1066 status byte. + * @return reasonCode Selector of the blocking error. + * @return details Encoded error data (empty on success). + */ + function _isAccountCompliant( + address account + ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) { + if (account != address(0)) { + (status, statusCode, reasonCode, details) = _validateAccountForTransfer(account, abi.encode(account)); + if (!status) return (status, statusCode, reasonCode, details); + } + return (true, Eip1066.SUCCESS, bytes32(0), EMPTY_BYTES); + } + /** * @notice Validates compliance for all parties involved in a transfer * or redemption. * @dev Checks the sender (if flagged), `from`, and `to` against * recovery, control list, and compliance contract. The compliance - * contract is called via staticcall with `canTransfer`. + * contract is called via static call with `canTransfer`. * @param from Source address (may be zero). * @param to Destination address (may be zero). * @param value Transfer amount. @@ -400,19 +473,13 @@ library ERC1594StorageWrapper { bool checkSender ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) { if (checkSender) { - (status, statusCode, reasonCode, details) = _validateAccountForTransfer(sender, abi.encode(sender)); - if (!status) return (status, statusCode, reasonCode, details); - (status, statusCode, reasonCode, details) = _validateSenderCompliance(sender, from, to, value); - if (!status) return (status, statusCode, reasonCode, details); - } - if (from != address(0)) { - (status, statusCode, reasonCode, details) = _validateAccountForTransfer(from, abi.encode(from)); - if (!status) return (status, statusCode, reasonCode, details); - } - if (to != address(0)) { - (status, statusCode, reasonCode, details) = _validateAccountForTransfer(to, abi.encode(to)); + (status, statusCode, reasonCode, details) = _isSenderCompliant(from, to, value, sender); if (!status) return (status, statusCode, reasonCode, details); } + (status, statusCode, reasonCode, details) = _isAccountCompliant(from); + if (!status) return (status, statusCode, reasonCode, details); + (status, statusCode, reasonCode, details) = _isAccountCompliant(to); + if (!status) return (status, statusCode, reasonCode, details); (status, statusCode, reasonCode, details) = _validateTransferCompliance(from, to, value); if (!status) return (status, statusCode, reasonCode, details); return (true, Eip1066.SUCCESS, bytes32(0), EMPTY_BYTES); diff --git a/packages/ats/contracts/contracts/domain/orchestrator/ClearingOps.sol b/packages/ats/contracts/contracts/domain/orchestrator/ClearingOps.sol index 062981556b..0face96329 100644 --- a/packages/ats/contracts/contracts/domain/orchestrator/ClearingOps.sol +++ b/packages/ats/contracts/contracts/domain/orchestrator/ClearingOps.sol @@ -425,7 +425,7 @@ library ClearingOps { // Verify identity and compliance for transfers to different addresses TokenCoreOps.checkIdentity(_id.tokenHolder, transferData.destination); - TokenCoreOps.checkCompliance(_id.tokenHolder, transferData.destination, false); + TokenCoreOps.checkCompliance(_id.tokenHolder, transferData.destination, transferData.amount, false); // Notify compliance module (same pattern as HoldStorageWrapper and ERC1410StorageWrapper) if (_id.partition == _DEFAULT_PARTITION && ERC3643StorageWrapper.erc3643Storage().compliance != address(0)) { @@ -466,7 +466,7 @@ library ClearingOps { // Approve: _verify identity/compliance (tokens are burned, no transfer back) TokenCoreOps.checkIdentity(_id.tokenHolder, address(0)); - TokenCoreOps.checkCompliance(_id.tokenHolder, address(0), false); + TokenCoreOps.checkCompliance(_id.tokenHolder, address(0), 0, false); } /** diff --git a/packages/ats/contracts/contracts/domain/orchestrator/TokenCoreOps.sol b/packages/ats/contracts/contracts/domain/orchestrator/TokenCoreOps.sol index def80597b0..f917bfbf8d 100644 --- a/packages/ats/contracts/contracts/domain/orchestrator/TokenCoreOps.sol +++ b/packages/ats/contracts/contracts/domain/orchestrator/TokenCoreOps.sol @@ -152,8 +152,8 @@ library TokenCoreOps { ERC1594StorageWrapper.checkIdentity(_from, _to); } - function checkCompliance(address _from, address _to, bool _checkSender) public view { - ERC1594StorageWrapper.checkCompliance(_from, _to, _checkSender); + function checkCompliance(address _from, address _to, uint256 amount, bool _checkSender) public view { + ERC1594StorageWrapper.checkCompliance(_from, _to, amount, _checkSender); } // Internal functions (inlined into calling StorageWrappers) diff --git a/packages/ats/contracts/contracts/facets/batchTransfer/BatchTransfer.sol b/packages/ats/contracts/contracts/facets/batchTransfer/BatchTransfer.sol index 131142dc32..db60bf1123 100644 --- a/packages/ats/contracts/contracts/facets/batchTransfer/BatchTransfer.sol +++ b/packages/ats/contracts/contracts/facets/batchTransfer/BatchTransfer.sol @@ -30,12 +30,13 @@ abstract contract BatchTransfer is IBatchTransfer, Modifiers { onlyUnProtectedPartitionsOrWildCardRole onlyClearingDisabled onlyIdentifiedAddresses(EvmAccessors.getMsgSender(), address(0)) - onlyCompliant(EvmAccessors.getMsgSender(), address(0), false) + onlyAccountCompliant(EvmAccessors.getMsgSender()) { uint256 length = _toList.length; for (uint256 i; i < length; ) { ERC1594StorageWrapper.checkIdentity(address(0), _toList[i]); - ERC1594StorageWrapper.checkCompliance(address(0), _toList[i], false); + ERC1594StorageWrapper.checkAccountCompliance(_toList[i]); + ERC1594StorageWrapper.checkTransferCompliance(EvmAccessors.getMsgSender(), _toList[i], _amounts[i]); unchecked { ++i; } diff --git a/packages/ats/contracts/contracts/services/asset/ComplianceModifiers.sol b/packages/ats/contracts/contracts/services/asset/ComplianceModifiers.sol index 08719af7eb..7a571a6751 100644 --- a/packages/ats/contracts/contracts/services/asset/ComplianceModifiers.sol +++ b/packages/ats/contracts/contracts/services/asset/ComplianceModifiers.sol @@ -64,7 +64,18 @@ abstract contract ComplianceModifiers { * @param checkSender Whether to also validate msg.sender against compliance rules. */ modifier onlyCompliant(address from, address to, bool checkSender) { - ERC1594StorageWrapper.checkCompliance(from, to, checkSender); + ERC1594StorageWrapper.checkCompliance(from, to, 0, checkSender); + _; + } + + /** + * @dev Modifier that verifies whether account satisfies + * all compliance rules enforced by the compliance module. + * + * @param account The account address to validate. + */ + modifier onlyAccountCompliant(address account) { + ERC1594StorageWrapper.checkAccountCompliance(account); _; } } diff --git a/packages/ats/contracts/test/contracts/integration/batchTransfer/batchTransfer.test.ts b/packages/ats/contracts/test/contracts/integration/batchTransfer/batchTransfer.test.ts index 01822a8201..aef2493c57 100644 --- a/packages/ats/contracts/test/contracts/integration/batchTransfer/batchTransfer.test.ts +++ b/packages/ats/contracts/test/contracts/integration/batchTransfer/batchTransfer.test.ts @@ -80,6 +80,10 @@ describe("BatchTransfer Tests", () => { role: ATS_ROLES.PROTECTED_PARTITIONS_ROLE, members: [signer_A.address], }, + { + role: ATS_ROLES.CONTROL_LIST_ROLE, + members: [signer_A.address], + }, ]); await asset.grantRole(ATS_ROLES.ISSUER_ROLE, signer_A.address); @@ -217,6 +221,58 @@ describe("BatchTransfer Tests", () => { "ComplianceNotAllowed", ); }); + + describe("ControlList", () => { + it("GIVEN a blacklisted sender WHEN batchTransfer THEN transaction fails with AccountIsBlocked", async () => { + await asset.addToControlList(signer_E.address); + + const toList = [signer_F.address]; + const amounts = [transferAmount]; + + await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError( + asset, + "AccountIsBlocked", + ); + }); + + it("GIVEN a blacklisted destination WHEN batchTransfer THEN transaction fails with AccountIsBlocked", async () => { + await asset.addToControlList(signer_F.address); + + const toList = [signer_D.address, signer_F.address]; + const amounts = [transferAmount, transferAmount]; + + await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError( + asset, + "AccountIsBlocked", + ); + }); + }); + + describe("Recovery", () => { + it("GIVEN a recovered sender WHEN batchTransfer THEN transaction fails with WalletRecovered", async () => { + await asset.recoveryAddress(signer_E.address, signer_D.address, ethers.ZeroAddress); + + const toList = [signer_F.address]; + const amounts = [transferAmount]; + + await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError( + asset, + "WalletRecovered", + ); + }); + + it("GIVEN a recovered destination WHEN batchTransfer THEN transaction fails with WalletRecovered", async () => { + await asset.recoveryAddress(signer_F.address, signer_D.address, ethers.ZeroAddress); + + const toList = [signer_F.address]; + const amounts = [transferAmount]; + + await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError( + asset, + "WalletRecovered", + ); + }); + }); }); });