Feat/delayed close by merisman · Pull Request #20 · MetaLex-Tech/cybercorps-contracts

merisman · 2025-04-08T20:02:05Z

No description provided.

gpxl-dev

See comments, a few subjective.

Most critical is that I don't think proposeAndSign can work?
Did not review tests.

gpxl-dev · 2025-04-08T20:23:18Z

        address[] memory parties,
        string[] memory creatingPartyValues,
-        string[] memory counterPartyValues
+        string[] memory counterPartyValues,


This assumes two parties - I wonder if we should account for more?

don't think so for dealmanager, escrow will only be for 2 parties.

gpxl-dev · 2025-04-08T20:32:25Z

+        string[] memory counterPartyValues,
+        bytes32 secretHash,
+        address finalizer
    ) external returns (bytes32 contractId) {


I think a neater implementation would be:

have createContract always accept an array of arrays of party values: string[][] partyValues

check every array inside partyValues is the correct length, and check that the corresponding party is not the zero address.

loop through partyValues[] setting the mapping per the parties

This way you only have a single createContract function. If you wanted to have two explicity entry points, you could make it internal and call it _createContract, and keep the two functions you have that call through appropriately.

Yes, have made the string[][] param throughout and kept it all one create method.

gpxl-dev · 2025-04-08T20:38:20Z


+    mapping(bytes32 => address[]) public voidedBy;
+
+    mapping(bytes32 => uint256) public expiry;


This expiry never gets set AFAICT.

Is there a good reason that voidedBy and expiry are not in the AgreementData struct?

also for voidedBy if moved AgreementData, I prefer `mapping(address => bool) voidRequested.

Added expiry (and secretHash) but kept the voidRequestedBy mapping outside -- I think this is probably preferable than nested mappings/arrays.

alright. We still have signedAt as a mapping though, just poitning out it's inconsistent really

gpxl-dev · 2025-04-08T21:10:39Z

+        if(!isParty(contractId, party)) revert NotAParty();
+
+        AgreementData storage agreementData = agreements[contractId];
+        if (agreementData.finalized) revert ContractAlreadyFinalized();


Since finalization is optional, I think we should probably check if there finalizer is a zero address, and if so, disallow voiding if the number of signatures is equal to the number of parties (i.e. it is fully signed)

That way you can't void if finalization of the contract is simply that it is fully signed.

I now set the finalized flag if finalizer is set to 0 and the contract is fully signed on the last signature.

gpxl-dev · 2025-04-08T21:16:11Z

            if (keccak256(abi.encode(counterPartyCheck)) != keccak256(abi.encode(partyValues))) revert CounterPartyValueMismatch();
        }

+        if(!conditionCheck(agreementId)) revert AgreementConditionsNotMet();


currently nowhere to set conditions

We need to allow these to be set on creation to remove any chance of signing before they are added. Also added editing by the issuer if the deal is pending.

gpxl-dev · 2025-04-08T21:20:27Z

            CORP,
            address(DEAL_REGISTRY),
-            _fillUnallocated
+            false


can remove this bool from the event I think - don't think it needs to be in here?

gpxl-dev · 2025-04-08T21:28:21Z

+        if(deal.status != EscrowStatus.PAID) revert EscrowNotPaid();
+        if(!ICyberDealRegistry(DEAL_REGISTRY).isVoided(agreementId)) revert DealNotVoided();
+
+        // Refund buyer assets first


Feels like something should be done with the corpAssets... I know in the case of the safe it will be burned, so this is ok for that... but wonder if we should still have the loop for corpassets regardless given we do in other functions.

Send them back to the printer contract, maybe? We won't burn the NFT's just void them.

gpxl-dev

See comments, a few subjective.

Most critical is that I don't think proposeAndSign can work?
Did not review tests.

Detoo

I need to think about @gpxl-dev 's comment on void workflow so flushing what I have for now

Detoo · 2025-04-08T22:09:10Z

        }
        voidEscrow(agreementId);
+        ICyberDealRegistry(DEAL_REGISTRY).voidContractFor(agreementId, signer, signature);
+    }


Missing expiry checks?

Not sure if it is intentional: it will make voidExpiredDeal exclusive to party members only. It used to be open to anyone

Updates: nvm, expiry checks is in voidContractFor();

however, what happens if none of the following conditions are met? (1) the number of void requests is equal to the number of parties, and (2) the expiry is in the past.

Wouldn't we end up with a weird state like this?

agreement.voided = false escrow.state = VOIDED

Detoo · 2025-04-09T17:07:01Z

        }
        voidEscrow(agreementId);
+        ICyberDealRegistry(DEAL_REGISTRY).voidContractFor(agreementId, signer, signature);
+    }


Updates: nvm, expiry checks is in voidContractFor();

however, what happens if none of the following conditions are met? (1) the number of void requests is equal to the number of parties, and (2) the expiry is in the past.

Wouldn't we end up with a weird state like this?

agreement.voided = false escrow.state = VOIDED

Detoo

I missed the merge of closed/open offers. A few minor suggestions

Detoo · 2025-04-09T18:49:00Z

+            }
+            //matching address cannot be 0
+            if (parties[i] == address(0)) {
+                revert FirstPartyZeroAddress();


Suggest refactoring it to PartyZeroAddress(uint256 index) due to its more general usage

Detoo · 2025-04-09T22:22:32Z

+        }
+
+        if(agreementData.voided)
+            emit ContractVoided(contractId, voidRequestedBy[contractId], block.timestamp);


shouldn't it revert if agreementData not voided? Otherwise we'd end up with a non-void agreement + a voided escrow

Major corrections after second-pass exploration of both repos: §5 Conditions — rewritten. Verified existing condition primitives: - BaseCondition, OrCondition, LexChexCondition (src/libs/conditions/) - NonUSNationalityCondition (full zkPassport + OFAC + founder override) - IssuerApprovalRecertificationCondition (pattern for GPLPApproval) LeXcheX fully implemented (src/creds/lexchex.sol + lexchexMinter.sol): Accreditation struct already has investorName/investorType/jurisdiction/ expiryDate; soulbound (ERC5484); no LeXcheXAdapter or LeXcheXNameLookup needed — use ILexChex directly. Legion Soulbound is a parallel LeXcheX deployment under Legion's operator key, not a new contract type. Removed the 20-item "new conditions" list; replaced with a verified- existing primitives table plus a narrower new-conditions list. §10 Webapp Layer — reframed. cyberSign (/cybersign/create and /[chainId]/ [agreementId]) is the correct precedent for trade-agreement creation and signing, not CyTE/LeXscroW Propose. CyTE renders a static IPFS doc and does NOT use the CyberAgreementRegistry template-with-globalFields/ partyFields machinery cyberTRADE needs. CyTE remains the precedent for escrow deposit + settlement (§10.4) only. Surfaced useFormSession multi- step pattern, useDealsForX hook family, useNotifications aggregator, useUploadPdfToPinata (Pinata is the IPFS provider), and PublicRoundsList as the Covered-UI-Provider-compliant listings precedent. §3.6 — added hooks-vs-conditions distinction table to clarify the two orthogonal gating layers. §7 — referenced existing template-registration scripts (template.s.sol, templatev2.s.sol, add-spa-plus-templates.s.sol) and existing templates/ directory; confirmed delegation already wired via Delegation struct + useCyberAgreementRegistryDelegations. §10.6 expanded — Officer / BorgAuth admin UI is a real gap (events indexed, no UI today). cyberTRADE BorgAuth role rotation (SECONDARY_ TRANSFER_ROLE on each SPV's DealManager) requires this admin UI to be built. Added §10.7 for per-cyberCORP disclosure documents tab (needed by spec §4.3.1; legacy documents system is Borg-only). §11 Indexer — CRITICAL gap: CyberAgreementRegistry events are NOT in Ponder config today. Added as discrete §14 step. Specified Pinata for IPFS. Added optional denormalized endorsement table. §12 — noted MetalexIssuerFeeHook exists as scrip-AMM groundwork; existing CyberScrip.sol has more compliance infrastructure than the "future enhancement" framing implied. §13 Discrepancies — added six new entries (#15-#20): MPC infrastructure absent (spec assumes), cap-table import absent (spec assumes), per-cyberCORP disclosure absent (spec assumes), spec's "existing FIX integration on issuer side" reference is unverifiable (none found), CyberShares.sol exists as partial implementation (decision gate before FundInterestExtension build), storage namespacing discipline as quasi-spec requirement. Also updated §13.5 — CyTE-as-precedent correction. cyberSign is the correct precedent for agreement creation; CyTE is the correct precedent for escrow only. §14 Sequence — restructured into three parallel workstreams (protocol / operational / application). Added discrete steps for: MPC integration (step 15), per-cyberCORP documents tab (step 17), cap-table import workflow (step 18), Officer/BorgAuth admin UI (step 19), per-SPV settings panel (step 20), FundSPVFactory extending PumpCorpFactory (step 12), CyberAgreementRegistry event handlers in Ponder (step 13). 23 items total, up from 15. §15 Anchored Pointers — added newly surfaced files: lexchex core, existing conditions primitives, MetalexIssuerFeeHook, PumpCorpFactory, CyberShares.sol, useFormSession, cyberSign routes, useNotifications, useUploadPdfToPinata, useRoundConditionsState, PublicRoundsList, ponder.config.ts, EvmProviders.tsx.

merisman added 8 commits April 6, 2025 11:31

Updating Close Flow, condition checks

8d12549

More delayed close methods

3961384

Updating cyber registry for handling the different void/cancel cases

0d1e81a

Finalizing CyberCerts, Adding more state checks

f8c06b3

More state checks.

d462c2f

More state and updates to lexscrow lite

cdd7354

cert ID in deal manager fix

da9a97d

Adding secret, adding tests, fixing void flows.

6701b50

merisman requested a review from Detoo April 8, 2025 20:02

gpxl-dev approved these changes Apr 8, 2025

View reviewed changes

gpxl-dev requested changes Apr 8, 2025

View reviewed changes

Detoo reviewed Apr 8, 2025

View reviewed changes

merisman added 5 commits April 9, 2025 00:02

Added string[][] for partyValues throughout, other pr fixes.

4004282

adding auth on add/remove conditions

2a9177c

Updates to voiding, adding timestamped event on void

3cb1722

Adding finalization events.

fce5c0f

Renaming CyberDealRegistry => CyberAgreementRegistry

89d0945

Detoo reviewed Apr 9, 2025

View reviewed changes

Updates to voidRequestedBy, adding counterparty values

973108c

Detoo reviewed Apr 9, 2025

View reviewed changes

merisman and others added 4 commits April 10, 2025 16:07

Updating Void Signature

305322a

Event tweaks

93895d5

Fix toJSON function

7d09c00

Skip duplicate revert for multiple zeroAddress parties

d88409a


		mapping(bytes32 => address[]) public voidedBy;

		mapping(bytes32 => uint256) public expiry;

Conversation

merisman commented Apr 8, 2025

Uh oh!

gpxl-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gpxl-dev Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gpxl-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Detoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Detoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gpxl-dev Apr 8, 2025 •

edited

Loading