Redesign ShareExtension and harden extension architecture#87
Redesign ShareExtension and harden extension architecture#87
Conversation
Implement comprehensive architectural redesign per DGCL compliance and NVCA fidelity requirements: - Split data model into SeriesTerms (canonical, stored once per series) and CertificateData (per-token, referencing series via seriesId) - Add series registry with create/update/targeted-update functions - Add per-certificate legend management (add/remove/initialize) for DGCL section 202 transfer restriction enforceability - Add issuer name as contract-level state (single-write name changes) - Replace closed ShareClass enum with extensible bytes32 shareClassKey - Add liquidation seniority ranking, expanded dividend parameters, conversion target series, mandatory conversion triggers, matter-specific special voting rights, stackable transfer restrictions with full legal text, and expanded redemption types - Add comprehensive validation invariants for series terms and cert data - Add events for series lifecycle, legend changes, and issuer updates - Support V1/V2 backward compatibility in supportsExtensionType and getExtensionURI with automatic format detection - Preserve UUPS storage layout (6 new state vars + 24 gap = 30 slots) https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
Add VotingScope enum (ClassWide, SeriesSpecific) to differentiate DGCL section 151(a) class-level votes (all Preferred voting together) from series-level votes (e.g., Series A voting alone). Add scope field to SpecialVotingRight struct, add hasSeriesVotingRights bool to SeriesTerms alongside existing hasClassVotingRights. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b35b5fa0a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restore Rule144Eligible at ordinal 3 and CustomRestriction at ordinal 4 to match the original V1 ABI encoding. Previously issued V1 certificates would have decoded to wrong restriction types after the ordinal shift. New types (LockUp, SecuritiesActRestriction) appended at ordinals 5-6. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
No V1 certificates exist in production, so ordinal preservation is unnecessary. Restore the clean enum ordering without the deprecated Rule144Eligible placeholder. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
Remove ShareClass enum, ShareData struct, V1 encode/decode functions, V1 URI builder, V1/V2 detection logic, and EXTENSION_TYPE_V2 constant. Simplify getExtensionURI to decode CertificateData directly. Rename internal _buildV2* helpers to _build* since there is only one version. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31b6cea66a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Phase 1 (HIGH priority): - Stock split mechanism with atomic price/share adjustment, split history, and batch support (recordStockSplit, recordStockSplitBatch, getSplitHistory) - Separate dynamic arrays from SeriesTerms into dedicated CRUD mappings: _conversionTriggers, _specialVotingRights, _transferRestrictions with add/remove/get/count functions for each - Series terms versioning: version counter, historical terms hashes, versioned update events Phase 2 (MEDIUM priority): - Expand MandatoryConversionTrigger with primaryThreshold, secondaryThreshold, additionalConditions for compound conditions - Add TransferRestrictionException sub-struct with exceptionType, exceptionText, requiresEvidence - Expand CertificateData with ShareRepresentationType (DGCL §158), holdingPeriodStartDate, holdingPeriodTackingApplied (Rule 144) - Add view helpers: getConversionRatio, getPaymentPercentage, computeAccruedDividends - Legend removal request audit trail (requestLegendRemoval + event) Phase 3 (LOW priority): - Add 8 MATTER_* constants for protective provisions (COI §3.3) - Add SECURITIES_ACT_LEGEND standard legend constant - Add stateOfIncorporation to issuer data (DGCL §158) - Add pagination helpers (getSeriesCount, getSeriesIdsPaginated) - Add NVCA optional fields to SeriesTerms (pay-to-play, registration rights, pro-rata, information rights, drag-along) Phase 4 (code quality): - Update URI builder to read from separated mappings - Update validation (pure where possible, removed stale trigger check from SeriesTerms since triggers are now separate) - Adjust storage layout: 13 slots used + __gap[17] = 30 total https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dcc26895e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add non-zero guard in updateConversionPrice() and in _validateSeriesTermsInternal() to prevent storing convertible terms with an undefined conversion ratio (OIP / 0). https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
The split handler was rescaling primaryThreshold for all trigger types, but only QualifiedIPO triggers store per-share price thresholds. ClassVote, DeemedLiquidation, and Custom triggers may store non-price values (vote percentages, aggregate amounts) that should not be adjusted. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
recordStockSplitBatch used this.recordStockSplit() which is an external call where msg.sender becomes the contract itself, causing the onlyOwner check to always revert. Extract the logic into an internal _recordStockSplit function called by both external entry points. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
_shareClassKeyToString previously collapsed all non-Common/Preferred keys to the literal "Custom", making distinct custom classes indistinguishable in metadata. Now renders the full bytes32 as a 0x-prefixed hex string so off-chain consumers can differentiate custom class identifiers. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
…asses When features are at their default/zero state (no liquidation preference, no dividends, not convertible, not redeemable, not partly paid), the URI builder now emits only the summary flag rather than all the detail fields. This keeps common stock metadata concise while preserving full detail for preferred series that actually use these features. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fccc4a22a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
For already-deployed upgradeable proxies, initialize() is not re-run after the hash-tracking upgrade, so defaultLegendHashes stays empty while defaultLegend has entries. This causes safeMint to copy mismatched arrays and removeDefaultLegendAt/removeCertLegendAt to revert on out-of-bounds hash reads. Fix: add _ensureDefaultLegendHashes() and _ensureCertLegendHashes() that lazily compute and append missing hashes on first mutation. Called from safeMint, safeMintAndAssign, removeDefaultLegendAt, and removeCertLegendAt. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5f8c7e1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
addDefaultLegend and addCertLegend were missing the lazy backfill guard, so on upgraded proxies the new hash would land at index 0 of an empty array while pre-existing legend texts occupied indices 0..N. This permanently corrupts hash-to-legend alignment since _ensure*Hashes() assumes the existing prefix is valid and only appends from hashLen. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 205cef4142
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
initializeLegends on ShareExtension copies series-specific restriction texts into newly minted certificates but was never called from any mint path, leaving share certificates without their legally required legends. Add _seedSeriesLegends() in IssuanceManager that extracts seriesId from extensionData, calls initializeLegends on the extension (try/catch for non-share extensions like SAFT/TokenWarrant), and invoke it from both createCert and createCertAndAssign. Also add getExtension to the ICyberCertPrinter interface. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5b61bb35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
createCertPrinter passes the extension address to the new CyberCertPrinter but never tells the extension about the cert printer, leaving certPrinter == address(0) on ShareExtension. This caused initializeLegends (and the mint-time _seedSeriesLegends) to always revert, silently swallowed by try/catch. Now createCertPrinter calls setCertPrinter on the extension after initialization (try/catch for non-share extensions that lack the function). https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbbf422008
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ShareExtension was never deployed to production, so the deprecated _deprecated_certificateLegends and _deprecated_certificateLegendHashes mappings (slots 3-4) served no purpose. Removed them and re-numbered storage slots. Added NatSpec annotation documenting no prior production deployment so future reviewers know upgrade-compat constraints don't apply. https://claude.ai/code/session_01VKgobkdkBH4e2bGify38vT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f37ce4a074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Redesign ShareExtension and harden extension architecture
Summary
SeriesTerms(class/series-level economics) and per-certificate data, with a full series registry, structured enums for voting rights, dividends, conversion, anti-dilution, and transfer restrictionsonlyIssuanceManagerOrExtensionaccess controlconversionPricefor convertible series; only scale price-denominated triggers during stock splits; extract_recordStockSplitinternal function to fix batch self-call revert (this.fn()msg.sender issue); honordividendCompoundingflag with annual compound interest supportshareClassKeyidentity (render as hex instead of collapsing to "Custom")StringUtilslibrary foruint256ToString, adopted by ShareExtension, SAFTExtension, SAFTEExtension, and TokenWarrantExtensionFiles changed
src/storage/extensions/ShareExtension.solsrc/CyberCertPrinter.solsrc/storage/CyberCertPrinterStorage.solcertLegendHashesanddefaultLegendHashesmappingssrc/libs/StringUtils.soluint256ToStringlibrarysrc/storage/extensions/SAFT*.sol,TokenWarrant*.solStringUtils