Skip to content

single pool approach #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gpxl-dev wants to merge 1 commit into
base: feat/partial-scripify
Choose a base branch
from
Open

single pool approach #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions CHANGESET.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# Changeset: Remove redundant scrip pool, simplify to single cert-decay pool

## Motivation

The current scrip accounting uses **two** MasterChef-style lazy-reduction pools:

1. **Scrip Pool** (`ScripPoolState` / `ScripUserInfo`) — tracks per-user scrip amounts with socialized reduction
2. **Cert Scrip Units Pool** (`CertScripUnitPool` / `CertScripState`) — tracks per-cert scripified units with socialized reduction

The scrip pool is redundant. Per-user scrip amounts are never read during conversion logic — the system uses real ERC-20 balances (`burnFrom`) instead. The `totalTrackedScrip` is always equal to `scrip.totalSupply()`. The per-user tracked amounts are informational only and decay in a way that is confusing to consumers.

The cert scrip units pool is the one that does real work: tracking how each cert's scripified units decay as others recertify, and enabling the "consume own first, socialize remainder" pattern.

## Changes

### 1. `src/storage/IssuanceManagerStorage.sol` — Storage structs

**Delete** the following (no longer needed):
- `ScripPoolState` struct (L143-146)
- `ScripUserInfo` struct (L153-156)
- `scripPoolStates` mapping from `IssuanceManagerData` (L121)
- `scripPoolUsers` mapping from `IssuanceManagerData` (L122)

**Keep** unchanged:
- `CertScripUnitPool`, `CertScripState` — the single remaining pool
- `ACC_REDUCTION_PRECISION` — still used by the cert pool
- `_currentAmount()` — still used by the cert pool

### 2. `src/storage/IssuanceManagerStorage.sol` — Delete scrip pool functions

**Delete entirely** (6 functions):
- `getScripPoolState()` (L342-346)
- `getScripPoolUserInfo()` (L348-353)
- `getScripPoolUserAmount()` (L355-362)
- `getScripPoolUserPosition()` (L364-376)
- `_depositScripPool()` (L916-929)
- `_reduceScripPool()` (L931-939)
- `_syncUserScripPoolPosition()` (L1001-1012)

### 3. `src/storage/IssuanceManagerStorage.sol` — Modify `getScripPoolTotals` (L432-442)

Replace the current implementation that reads from `ScripPoolState` with a simple read of scrip `totalSupply()`:

```solidity
function getScripPoolTotals(
address certAddress
) internal view returns (uint256 totalTrackedScrip) {
address scripAddress = getScripifiedCert(certAddress);
if (scripAddress == address(0)) return 0;
totalTrackedScrip = ICyberScrip(scripAddress).totalSupply();
}
```

This changes the return signature — it no longer returns `accReductionPerShare` (which belonged to the deleted scrip pool). Callers that used the second return value must be updated.

### 4. `src/storage/IssuanceManagerStorage.sol` — Modify `executeScripifyCert` (~L611-684)

Remove the call to `_depositScripPool` (L681). The ERC-20 `mint` on the next line already tracks the user's balance. No replacement needed.

Before:
```solidity
_depositScripPool(certAddress, account, scripAmount);
ICyberScrip(scripifiedCert).mint(toSend, scripAmount);
```

After:
```solidity
ICyberScrip(scripifiedCert).mint(toSend, scripAmount);
```

### 5. `src/storage/IssuanceManagerStorage.sol` — Modify `executeConvertScripToCert` (~L686-783)

Remove the call to `_reduceScripPool` (L737). The ERC-20 `burnFrom` a few lines later already reduces `totalSupply()`. No replacement needed.

Before:
```solidity
_reduceScripPool(certAddress, amount);
if (selection.foundActive) {
```

After:
```solidity
if (selection.foundActive) {
```

### 6. `src/storage/IssuanceManagerStorage.sol` — Modify `executeForceScripBurn` (~L786-807)

Remove the call to `_reduceScripPool` (L804). The `forceBurn` call already reduces `totalSupply()`.

Before:
```solidity
_reduceScripPool(certAddress, amount);
_reduceCertScripUnitsPool(certAddress, unitsWad);
ICyberScrip(scripifiedCert).forceBurn(account, amount);
```

After:
```solidity
_reduceCertScripUnitsPool(certAddress, unitsWad);
ICyberScrip(scripifiedCert).forceBurn(account, amount);
```

### 7. `src/IssuanceManager.sol` — Facade functions

**Modify** `getScripPoolTotals` (L914-922) to match the new return signature (single return value, no `accReductionPerShare`).

**Delete** `getScripPoolUserAmount` (L934-939) and `getScripPoolUserPosition` (L941-950) — these have no backing storage anymore. (Note: these are NOT declared in `IIssuanceManager.sol`, so no interface change needed.)

### 8. Tests — `test/IssuanceManagerConversionTest.t.sol`

Update assertions that use `getScripPoolTotals` to use the new single-return-value signature. The value itself (`totalTrackedScrip`) should still be correct since it now reads `totalSupply()`.

In the invariant assertions, the pattern:
```solidity
(totalTrackedScrip,) = issuanceManager.getScripPoolTotals(address(certPrinter));
assertEq(totalScripifiedUnits, totalTrackedScrip); // THIS IS THE FAILING ASSERTION
assertEq(totalActiveUnits + totalTrackedScrip, 400);
```

The second assertion (`activeUnits + totalTrackedScrip == 400`) will still hold — `totalSupply()` is exact.

The first assertion (`totalScripifiedUnits == totalTrackedScrip`) compares the sum of per-cert lazy-reduced values against the pool total. This will still have MasterChef rounding. Change to `<=`:
```solidity
assertLe(totalScripifiedUnits, totalTrackedScrip);
```

### 9. Tests — `test/CyberScripUpgradeTest.t.sol`

- Update `getScripPoolTotals` calls to new signature.
- **Delete** all `getScripPoolUserAmount` assertions (L802-814, L829-841, L860-872) — these functions no longer exist.

## What NOT to change

- `CertScripUnitPool`, `CertScripState`, and all cert-pool functions (`_depositCertScripUnits`, `_reduceCertScripUnitsPool`, `_consumeOwnCertScripUnits`, `_syncCertScripPosition`, `getCurrentCertScripifiedUnits`) — these are the single remaining pool and stay as-is.
- `_currentAmount()`, `ACC_REDUCTION_PRECISION` — shared helpers, still used.
- `IIssuanceManager.sol` — the deleted facade functions were never in the interface.
- `CyberScrip.sol` — no changes needed.
- The "consume own first, socialize remainder" pattern in `executeConvertScripToCert` — unchanged.
18 changes: 5 additions & 13 deletions src/IssuanceManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,27 +916,19 @@ contract IssuanceManager is Initializable, BorgAuthACL, UUPSUpgradeable {
)
external
view
returns (uint256 totalTrackedScrip, uint256 accReductionPerShare)
returns (uint256 totalTrackedScrip)
{
return IssuanceManagerStorage.getScripPoolTotals(certAddress);
}

function getScripPoolUserAmount(
address certAddress,
address account
) external view returns (uint256) {
return IssuanceManagerStorage.getScripPoolUserAmount(certAddress, account);
}

function getScripPoolUserPosition(
address certAddress,
address account
function getCertScripUnitPoolTotals(
address certAddress
)
external
view
returns (uint256 recordedAmount, uint256 reductionDebt, uint256 currentAmount)
returns (uint256 totalScripifiedUnits, uint256 accReductionPerShare)
{
return IssuanceManagerStorage.getScripPoolUserPosition(certAddress, account);
return IssuanceManagerStorage.getCertScripUnitPoolTotals(certAddress);
}

function isScripifyWhitelisted(
Expand Down
103 changes: 11 additions & 92 deletions src/storage/IssuanceManagerStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ library IssuanceManagerStorage {
mapping(address => mapping(uint256 => bool)) scripifyWhitelist;
mapping(address => mapping(uint256 => CertScripState)) certScripStates;
mapping(address => CertScripUnitPool) certScripUnitPools;
mapping(address => ScripPoolState) scripPoolStates;
mapping(address => mapping(address => ScripUserInfo)) scripPoolUsers;
mapping(address => mapping(address => RecertificationApproval))
recertificationApprovals;
Comment on lines 119 to 122
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the IssuanceManager storage layout

Because IssuanceManager is upgraded in place (src/IssuanceManager.sol is UUPS-upgradeable, and test/CyberScripUpgradeTest.t.sol exercises proxy upgrades), deleting the two mapping fields above changes the storage slot of recertificationApprovals from the previous implementation. After upgrading an existing proxy, any approvals written before the upgrade will be read from the wrong slot, so pending conversions that previously had approval can start reverting with RecertificationApprovalRequired. Leave the retired mappings in place as unused storage gaps, or add an explicit migration, instead of reordering IssuanceManagerData.

Useful? React with 👍 / 👎.

}
Expand All @@ -140,21 +138,11 @@ library IssuanceManagerStorage {
uint256 maxUnitsRepresented;
}

struct ScripPoolState {
uint256 totalTrackedScrip;
uint256 accReductionPerShare;
}

struct CertScripUnitPool {
uint256 totalScripifiedUnits;
uint256 accReductionPerShare;
}

struct ScripUserInfo {
uint256 amount;
uint256 reductionDebt;
}

struct RecertificationApproval {
bool approved;
string investorName;
Expand Down Expand Up @@ -339,42 +327,6 @@ library IssuanceManagerStorage {
return issuanceManagerStorage().certScripStates[certAddress][id];
}

function getScripPoolState(
address certAddress
) internal view returns (ScripPoolState storage) {
return issuanceManagerStorage().scripPoolStates[certAddress];
}

function getScripPoolUserInfo(
address certAddress,
address account
) internal view returns (ScripUserInfo storage) {
return issuanceManagerStorage().scripPoolUsers[certAddress][account];
}

function getScripPoolUserAmount(
address certAddress,
address account
) internal view returns (uint256) {
ScripPoolState storage pool = getScripPoolState(certAddress);
ScripUserInfo storage user = getScripPoolUserInfo(certAddress, account);
return _currentAmount(user.amount, user.reductionDebt, pool.accReductionPerShare);
}

function getScripPoolUserPosition(
address certAddress,
address account
)
internal
view
returns (uint256 recordedAmount, uint256 reductionDebt, uint256 currentAmount)
{
ScripUserInfo storage user = getScripPoolUserInfo(certAddress, account);
recordedAmount = user.amount;
reductionDebt = user.reductionDebt;
currentAmount = getScripPoolUserAmount(certAddress, account);
}

function getRecertificationApproval(
address certAddress,
address investor
Expand Down Expand Up @@ -431,13 +383,21 @@ library IssuanceManagerStorage {

function getScripPoolTotals(
address certAddress
) internal view returns (uint256 totalTrackedScrip) {
address scripAddress = getScripifiedCert(certAddress);
if (scripAddress == address(0)) return 0;
totalTrackedScrip = ICyberScrip(scripAddress).totalSupply();
}

function getCertScripUnitPoolTotals(
address certAddress
)
internal
view
returns (uint256 totalTrackedScrip, uint256 accReductionPerShare)
returns (uint256 totalScripifiedUnits, uint256 accReductionPerShare)
{
ScripPoolState storage pool = getScripPoolState(certAddress);
totalTrackedScrip = pool.totalTrackedScrip;
CertScripUnitPool storage pool = issuanceManagerStorage().certScripUnitPools[certAddress];
totalScripifiedUnits = pool.totalScripifiedUnits;
accReductionPerShare = pool.accReductionPerShare;
}

Expand Down Expand Up @@ -666,7 +626,6 @@ library IssuanceManagerStorage {
_depositCertScripUnits(certAddress, id, amountWad);
details.unitsRepresented = details.unitsRepresented - amountWad;
certificate.updateCertificateDetails(id, details);
_depositScripPool(certAddress, account, scripAmount);
ICyberScrip(scripifiedCert).mint(toSend, scripAmount);
emit ScripifiedCert(certAddress, id, scripifiedCert, amount);
}
Expand Down Expand Up @@ -722,7 +681,6 @@ library IssuanceManagerStorage {
approval.details = approvedDetails;
}

_reduceScripPool(certAddress, amount);
if (selection.foundActive) {
uint256 pooledUnitsWad = _consumeOwnCertScripUnits(
certAddress,
Expand Down Expand Up @@ -789,7 +747,6 @@ library IssuanceManagerStorage {
units = units / numerator;
uint256 unitsWad = units * 1e18;

_reduceScripPool(certAddress, amount);
_reduceCertScripUnitsPool(certAddress, unitsWad);
ICyberScrip(scripifiedCert).forceBurn(account, amount);
}
Expand Down Expand Up @@ -901,31 +858,6 @@ library IssuanceManagerStorage {
}
}

function _depositScripPool(
address certAddress,
address account,
uint256 scripAmount
) internal {
ScripPoolState storage pool = getScripPoolState(certAddress);
ScripUserInfo storage user = getScripPoolUserInfo(certAddress, account);
_syncUserScripPoolPosition(certAddress, account);
pool.totalTrackedScrip = pool.totalTrackedScrip + scripAmount;
user.amount = user.amount + scripAmount;
user.reductionDebt =
(user.amount * pool.accReductionPerShare) /
ACC_REDUCTION_PRECISION;
}

function _reduceScripPool(address certAddress, uint256 burnedScripAmount) internal {
ScripPoolState storage pool = getScripPoolState(certAddress);
if (burnedScripAmount > pool.totalTrackedScrip) revert ConditionCheckFailed();
if (pool.totalTrackedScrip == 0) revert ConditionCheckFailed();
pool.accReductionPerShare =
pool.accReductionPerShare +
((burnedScripAmount * ACC_REDUCTION_PRECISION) / pool.totalTrackedScrip);
pool.totalTrackedScrip = pool.totalTrackedScrip - burnedScripAmount;
}

function _depositCertScripUnits(
address certAddress,
uint256 tokenId,
Expand Down Expand Up @@ -986,19 +918,6 @@ library IssuanceManagerStorage {
return requestedUnits - directUnits;
}

function _syncUserScripPoolPosition(address certAddress, address account) internal {
ScripPoolState storage pool = getScripPoolState(certAddress);
ScripUserInfo storage user = getScripPoolUserInfo(certAddress, account);
user.amount = _currentAmount(
user.amount,
user.reductionDebt,
pool.accReductionPerShare
);
user.reductionDebt =
(user.amount * pool.accReductionPerShare) /
ACC_REDUCTION_PRECISION;
}

function _syncCertScripPosition(address certAddress, uint256 tokenId) internal {
CertScripUnitPool storage pool = issuanceManagerStorage().certScripUnitPools[
certAddress
Expand Down
Loading
Loading