Fix/secondary trading internal audit

src/DealManager.sol

@@ -89,6 +89,7 @@ contract DealManager is
     error ConditionDoesNotExist();
     error NotRefImplementation();
     event MinTradeThresholdSet(uint256 minUnits, uint256 minConsideration, address setter);
+    event SettlementWindowSet(uint256 window, address setter);
 
     /// @notice Maps agreement IDs to arrays of counter party values for closed deals.
     mapping(bytes32 => string[]) public counterPartyValues;
@@ -500,6 +501,11 @@ contract DealManager is
         emit MinTradeThresholdSet(units, consideration, msg.sender);
     }
 
+    function setSettlementWindow(uint256 window) external onlyAdmin {
+        SecondaryTradeStorage.secondaryTradeStorage().settlementWindow = window;
+        emit SettlementWindowSet(window, msg.sender);
+    }
+
     function setDefaultIntegrator(address integrator) external onlyAdmin {
         if (integrator != address(0)) {
             if (!IDealManagerFactory(DealManagerStorage.getUpgradeFactory()).isIntegratorWhitelisted(integrator))
@@ -560,6 +566,11 @@ contract DealManager is
         return SecondaryTradeStorage.secondaryTradeStorage().defaultIntegrator;
     }
 
+    /// @notice Effective settlement window (applies the default when unset).
+    function getSettlementWindow() external view returns (uint256) {
+        return SecondaryTradeStorage.getSettlementWindow();
+    }
+
     function getOffer(bytes32 offerId) external view returns (Offer memory) {
         return SecondaryTradeStorage.secondaryTradeStorage().offers[offerId];
     }

src/IssuanceManager.sol

@@ -553,6 +553,12 @@ contract IssuanceManager is Initializable, BorgAuthACL, UUPSUpgradeable {
         return IssuanceManagerStorage.getPrinters()[index];
     }
 
+    /// @notice Whether `printer` is a certificate printer this IssuanceManager created and still tracks
+    /// @dev Authoritative membership check against the registry; a self-reporting printer cannot forge it
+    function isPrinter(address printer) external view returns (bool) {
+        return IssuanceManagerStorage.isPrinter(printer);
+    }
+
     /// @notice Sets the URI builder contract address
     /// @dev Only callable by owner
     /// @param _uriBuilder New URI builder contract address

src/interfaces/IDealManager.sol

@@ -134,6 +134,8 @@ interface IDealManager {
     function cancelOffer(bytes32 offerAgreementId) external;
     function acceptOffer(AcceptOfferParams calldata params) external returns (bytes32 settlementAgreementId);
     function setMinTradeThreshold(uint256 units, uint256 consideration) external;
+    function setSettlementWindow(uint256 window) external;
+    function getSettlementWindow() external view returns (uint256);
     function setDefaultIntegrator(address integrator) external;
     function getOffer(bytes32 offerAgreementId) external view returns (Offer memory);
     function getSecondaryEscrow(bytes32 agreementId) external view returns (SecondaryEscrow memory);

src/interfaces/IIssuanceManager.sol

@@ -405,6 +405,7 @@ interface IIssuanceManager {
     function cyberCertPrinterBeacon() external view returns (UpgradeableBeacon);
     function cyberScripBeacon() external view returns (UpgradeableBeacon);
     function printers(uint256 index) external view returns (address);
+    function isPrinter(address printer) external view returns (bool);
     function setUriBuilder(address _uriBuilder) external;
 
     /// @notice Single-source signal for the buyer's newly minted Ledger Entry Token at secondary settlement.

src/interfaces/ISecondaryTradeStorage.sol

@@ -213,7 +213,9 @@ interface ISecondaryTradeStorage {
     error IntegratorNotWhitelisted();
     error UnitsExceedOffer();
     error NotOfferor();
+    error NotCertOwner();
     error MissingCertPrinter();
+    error UnknownCertPrinter();
     error NotPartyToAgreement();
     error OfferAlreadyExists();
     /// @notice Caller is not the signer they claim to be (signer must equal msg.sender)
@@ -228,6 +230,9 @@ interface ISecondaryTradeStorage {
     error SecondaryTradeAgreementExpired();
     error SecondaryTradeAgreementNotExpired();
     error SecondaryTradeAgreementNotVoided();
+    /// @notice At finalize, the cert's current legal owner no longer matches the settlement's seller of record
+    /// (ownership moved after listing/acceptance), so the payee and the party whose units are consumed diverge
+    error SecondaryTradeSellerOwnershipChanged();
     error SecondaryConditionsNotMet(address condition);
     /// @notice Condition address supplied to a config setter is the zero address
     error InvalidSecondaryCondition();

src/storage/IssuanceManagerStorage.sol

@@ -234,6 +234,16 @@ library IssuanceManagerStorage {
         return issuanceManagerStorage().printers;
     }
 
+    /// @dev Linear membership scan over the printer registry. The list is admin-curated and small, and it is
+    /// the only authoritative source of printers created by this IssuanceManager (mirrors removePrinter).
+    function isPrinter(address printer) internal view returns (bool) {
+        address[] storage printers = issuanceManagerStorage().printers;
+        for (uint256 i = 0; i < printers.length; i++) {
+            if (printers[i] == printer) return true;
+        }
+        return false;
+    }
+
     // Setters
     function setCORP(address _corp) internal {
         issuanceManagerStorage().CORP = _corp;

src/storage/SecondaryTradeStorage.sol

@@ -46,6 +46,7 @@ import "openzeppelin-contracts/token/ERC20/utils/SafeERC20.sol";
 import {ECDSA} from "openzeppelin-contracts/utils/cryptography/ECDSA.sol";
 import "../interfaces/ICyberAgreementRegistry.sol";
 import "../interfaces/IIssuanceManager.sol";
+import {ICyberCertPrinter} from "../interfaces/ICyberCertPrinter.sol";
 import "../interfaces/IDealManagerFactory.sol";
 import "../interfaces/IDealManager.sol";
 import "openzeppelin-contracts/utils/introspection/ERC165Checker.sol";
@@ -67,6 +68,12 @@ library SecondaryTradeStorage {
 
     bytes32 constant STORAGE_POSITION = keccak256("cybercorp.secondary.trade.storage.v1");
 
+    /// @dev Fallback settlement window used when settlementWindow is unset (0), so an accepted lot always
+    /// gets a finalize window measured from acceptance rather than inheriting the (possibly imminent) offer
+    /// expiry. Must exceed any configured TimeSettlementPeriodCondition minimum delay for the lot to be
+    /// finalizeable; owners with a longer minimum (e.g. QMS-mode) set a larger window via setSettlementWindow.
+    uint256 constant DEFAULT_SETTLEMENT_WINDOW = 7 days;
+
     // ── EIP-712 relayer-authorization constants (see the relayer overloads of post/cancel/acceptOffer) ──
     // The signed message binds the full structured params (so a wallet renders each named field), the
     // principal `forAddr`, and an unordered `nonce` that makes each authorization single-use independent
@@ -112,6 +119,15 @@ library SecondaryTradeStorage {
         address[] closingConditions;                             // default closing set
         // Consumed relayer-authorization nonces: usedAuthNonce[forAddr][nonce], order-independent single-use.
         mapping(address => mapping(uint256 => bool)) usedAuthNonce;
+        // Per-DealManager settlement window: how long after acceptance a lot has to finalize before it can be
+        // voided as expired. Decouples settlement expiry from offer expiry (0 = DEFAULT_SETTLEMENT_WINDOW).
+        uint256 settlementWindow;
+    }
+
+    /// @notice Effective settlement window, applying the default when unset (so legacy DealManager does not need migration)
+    function getSettlementWindow() internal view returns (uint256) {
+        uint256 window = secondaryTradeStorage().settlementWindow;
+        return window == 0 ? DEFAULT_SETTLEMENT_WINDOW : window;
     }
 
     function secondaryTradeStorage() internal pure returns (SecondaryTradeData storage ds) {
@@ -147,6 +163,8 @@ library SecondaryTradeStorage {
 
         // validate parameters
         if (params.certPrinter == address(0)) revert ISecondaryTradeStorage.MissingCertPrinter();
+        if (!DealManagerStorage.getIssuanceManager().isPrinter(params.certPrinter))
+            revert ISecondaryTradeStorage.UnknownCertPrinter();
 
         // Validate integrator
         address integrator = params.integrator != address(0)
@@ -219,6 +237,8 @@ library SecondaryTradeStorage {
         _checkThresholdConditions(offerId, bytes32(0));
 
         if (params.side == OfferSide.SELL) {
+            if (ICyberCertPrinter(params.certPrinter).legalOwnerOf(params.tokenId) != offeror)
+                revert ISecondaryTradeStorage.NotCertOwner();
             // Reserve units on the seller's cert (routed through IssuanceManager, the only caller the printer allows)
             DealManagerStorage.getIssuanceManager().increaseUnitsReserved(params.certPrinter, params.tokenId, params.units);
         } else {
@@ -328,6 +348,11 @@ library SecondaryTradeStorage {
             _checkMinTradeThreshold(remainderUnits, remainderConsideration);
         }
 
+        // Settlement window runs from acceptance, not the offer's validUntil: a lot accepted moments before
+        // the offer expires still gets the full window to finalize (and clear any settlement-period minimum
+        // delay), instead of a truncated or already-expired one.
+        uint256 settlementExpiry = block.timestamp + getSettlementWindow();
+
         // Create fully-signed settlement agreement via registry.
         // settlementAgreementIds.length is a push-only monotonic nonce: unique per acceptance even if prior
         // settlements are later voided (which decrements unitsAccepted but never shrinks the array).
@@ -347,7 +372,7 @@ library SecondaryTradeStorage {
             settlementPartyValues,
             bytes32(0),
             address(this),
-            offer.validUntil
+            settlementExpiry
         );
         // Offeror: DealManager (finalizer) attests commitment via signContractWithEscrow.
         // The registry does not verify escrowSigner's EIP-712 sig here; the offeror's
@@ -375,11 +400,16 @@ library SecondaryTradeStorage {
             tokenId = offer.tokenId;
             buyer = acceptor;
             endorsementSig = offer.openEndorsementSig;
+            // Re-check the seller still owns the cert
+            if (ICyberCertPrinter(certPrinter).legalOwnerOf(tokenId) != offer.offeror)
+                revert ISecondaryTradeStorage.SecondaryTradeSellerOwnershipChanged();
         } else {
             certPrinter = offer.certPrinter;
             tokenId = params.sellerTokenId;
             buyer = offer.offeror;
             endorsementSig = params.openEndorsementSig;
+            if (ICyberCertPrinter(certPrinter).legalOwnerOf(tokenId) != acceptor)
+                revert ISecondaryTradeStorage.NotCertOwner();
             // Reserve units on the seller's cert at acceptance (bid flow, routed through IssuanceManager)
             DealManagerStorage.getIssuanceManager().increaseUnitsReserved(certPrinter, tokenId, params.units);
         }
@@ -410,7 +440,7 @@ library SecondaryTradeStorage {
             paymentToken: offer.paymentToken,
             paymentAmount: partialConsideration,
             units: params.units,
-            expiry: offer.validUntil,
+            expiry: settlementExpiry,
             status: SecondaryEscrowStatus.ACCEPTED,
             feeDestination: offer.integrator,
             offerId: params.offerId,
@@ -442,7 +472,7 @@ library SecondaryTradeStorage {
         // Acceptance funds the escrow atomically, so this event carries the settlement's payment too.
         emit ISecondaryTradeStorage.OfferAccepted(
             params.offerId, settlementAgreementId, acceptor, params.units, offer.paymentToken, partialConsideration,
-            tokenId, offer.validUntil,
+            tokenId, settlementExpiry,
             buyerName, buyerHostingMode, adminMultisig, endorsementSig
         );
     }
@@ -505,6 +535,10 @@ library SecondaryTradeStorage {
             offer.status = OfferStatus.FINALIZED;
         }
         (address seller, address buyer) = _settlementParties(offer, secEscrow);
+        // Require seller ownership to remain unchanged; if it was a legitimately-moved
+        // position it'd still revert here and it could be resolved via the void/expiry path instead of mispaying.
+        if (ICyberCertPrinter(offer.certPrinter).legalOwnerOf(secEscrow.tokenId) != seller)
+            revert ISecondaryTradeStorage.SecondaryTradeSellerOwnershipChanged();
         // Fee math (mirrors DealManager.computeFee / getPlatformPayable) computed directly from the factory
         address upgradeFactory = DealManagerStorage.getUpgradeFactory();
         uint256 fee = secEscrow.paymentAmount * IDealManagerFactory(upgradeFactory).getDefaultFeeRatio() / DealManagerFactoryStorage.BASIS_POINTS;

test/DealManagerSecondaryTradeIndexerTest.t.sol

@@ -71,9 +71,9 @@ import {CertificateDetails, ICyberCertPrinter} from "../src/interfaces/ICyberCer
 // Reuse the payment-token mock and the minimal CyberCorp / uriBuilder fixtures from sibling test files.
 import {SecERC20Mock} from "./DealManagerSecondaryTradeTest.t.sol";
 import {
-    MockCyberCorpForCertEvent,
-    MockUriBuilderForCertEvent
-} from "./IssuanceManagerCertificateCreatedEventTest.t.sol";
+    MockCyberCorpForIM,
+    MockUriBuilderForIM
+} from "./IssuanceManagerTest.t.sol";
 
 /// @title DealManagerSecondaryTradeIndexerTest
 /// @notice Simulates an off-chain indexer (e.g. powering the Legion UI) and proves the secondary-trade
@@ -224,7 +224,7 @@ contract DealManagerSecondaryTradeIndexerTest is Test {
     ICyberCertPrinter public certPrinter;
     IssuanceManager public im;
     CyberAgreementRegistry public registry;
-    MockCyberCorpForCertEvent public corp;
+    MockCyberCorpForIM public corp;
     DealManagerFactory public dmFactory;
     DealManager public dm;
     BorgAuth public auth;
@@ -244,7 +244,7 @@ contract DealManagerSecondaryTradeIndexerTest is Test {
 
         paymentToken = new SecERC20Mock();
         auth = new BorgAuth(owner);
-        corp = new MockCyberCorpForCertEvent();
+        corp = new MockCyberCorpForIM();
 
         // Real IssuanceManager + CyberCertPrinter, deployed through the IssuanceManagerFactory beacon stack
         // (mirrors IssuanceManagerSecondaryTransferTest), so the secondary-transfer logs are the real ones.
@@ -263,7 +263,7 @@ contract DealManagerSecondaryTradeIndexerTest is Test {
             )
         );
         im = IssuanceManager(imFactory.deployIssuanceManager(imSalt));
-        im.initialize(address(auth), address(corp), address(new MockUriBuilderForCertEvent()), address(imFactory));
+        im.initialize(address(auth), address(corp), address(new MockUriBuilderForIM()), address(imFactory));
 
         registry = CyberAgreementRegistry(
             address(