Skip to content

Commit 01b12fe

Browse files
committed
fix: resolve coverage build logic error after contract changes
- Fix root test:coverage to use build:self:coverage for proper coverage artifacts - Add build:coverage chain in issuance package for coverage-specific compilation - Inline coverage script execution in test:coverage:self - Remove scripts/coverage file (logic moved inline) The original issue was that pnpm test:coverage reported incomplete coverage after contract changes, requiring manual pnpm clean. This was caused by coverage tests using regular build artifacts instead of coverage-specific artifacts. The fix ensures coverage builds use the correct configuration and generate proper artifacts automatically.
1 parent 538e004 commit 01b12fe

File tree

8 files changed

+172
-83
lines changed

8 files changed

+172
-83
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"lint:yaml": "npx yaml-lint .github/**/*.{yml,yaml} packages/contracts/task/config/*.yml; prettier -w --cache --log-level warn '**/*.{yml,yaml}'",
2121
"format": "prettier -w --cache --log-level warn '**/*.{js,ts,cjs,mjs,jsx,tsx,json,md,yaml,yml}'",
2222
"test": "pnpm build && pnpm -r run test:self",
23-
"test:coverage": "pnpm build && pnpm -r run test:coverage:self"
23+
"test:coverage": "pnpm build && pnpm -r run build:self:coverage && pnpm -r run test:coverage:self"
2424
},
2525
"devDependencies": {
2626
"@changesets/cli": "catalog:",

packages/issuance/contracts/allocate/DirectAllocation.sol

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,28 @@ import { ERC165Upgradeable } from "@openzeppelin/contracts-upgradeable/utils/int
1414
* @notice A simple contract that receives tokens from the IssuanceAllocator and allows
1515
* an authorized operator to withdraw them.
1616
*
17-
* @dev This contract is designed to be a non-self-minting target in the IssuanceAllocator.
17+
* @dev This contract is designed to be an allocator-minting target in the IssuanceAllocator.
1818
* The IssuanceAllocator will mint tokens directly to this contract, and the authorized
1919
* operator can send them to individual addresses as needed.
2020
*
2121
* This contract is pausable by the PAUSE_ROLE. When paused, tokens cannot be sent.
2222
* @custom:security-contact Please email [email protected] if you find any bugs. We might have an active bug bounty program.
2323
*/
2424
contract DirectAllocation is BaseUpgradeable, IIssuanceTarget {
25+
26+
// -- Custom Errors --
27+
28+
/// @notice Thrown when token transfer fails
29+
/// @param to The address that the transfer was attempted to
30+
/// @param amount The amount of tokens that failed to transfer
31+
error SendTokensFailed(address to, uint256 amount);
32+
2533
// -- Events --
2634

2735
/// @notice Emitted when tokens are sent
2836
/// @param to The address that received the tokens
2937
/// @param amount The amount of tokens sent
30-
event TokensSent(address indexed to, uint256 amount); // solhint-disable-line gas-indexed-events
38+
event TokensSent(address indexed to, uint256 indexed amount);
3139
// Do not need to index amount, ignoring gas-indexed-events warning.
3240

3341
/// @notice Emitted before the issuance allocation changes
@@ -72,9 +80,7 @@ contract DirectAllocation is BaseUpgradeable, IIssuanceTarget {
7280
* @param amount Amount of tokens to send
7381
*/
7482
function sendTokens(address to, uint256 amount) external onlyRole(OPERATOR_ROLE) whenNotPaused {
75-
// TODO: missed for audit, should change to custom error in furture
76-
// solhint-disable-next-line gas-custom-errors
77-
require(GRAPH_TOKEN.transfer(to, amount), "!transfer");
83+
require(GRAPH_TOKEN.transfer(to, amount), SendTokensFailed(to, amount));
7884
emit TokensSent(to, amount);
7985
}
8086

@@ -88,10 +94,8 @@ contract DirectAllocation is BaseUpgradeable, IIssuanceTarget {
8894
}
8995

9096
/**
97+
* @dev No-op for DirectAllocation; issuanceAllocator is not stored.
9198
* @inheritdoc IIssuanceTarget
9299
*/
93-
function setIssuanceAllocator(address issuanceAllocator) external virtual override onlyRole(GOVERNOR_ROLE) {
94-
// No-op for DirectAllocation
95-
// This contract doesn't need to store the issuance allocator
96-
}
100+
function setIssuanceAllocator(address issuanceAllocator) external virtual override onlyRole(GOVERNOR_ROLE) { }
97101
}

packages/issuance/contracts/allocate/IssuanceAllocator.md

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# IssuanceAllocator
22

3-
The IssuanceAllocator is a smart contract responsible for allocating token issuance to different components of The Graph protocol. It calculates issuance for all targets based on their configured proportions and handles minting for non-self-minting targets.
3+
The IssuanceAllocator is a smart contract responsible for allocating token issuance to different components of The Graph protocol. It calculates issuance for all targets based on their configured proportions and handles minting for allocator-minting targets.
44

55
## Overview
66

@@ -35,7 +35,7 @@ When the contract is paused:
3535

3636
- **Distribution stops**: `distributeIssuance()` returns early without minting any tokens, returning the last block when issuance was distributed.
3737
- **Accumulation begins**: Issuance for allocator-minting targets accumulates in `pendingAccumulatedAllocatorIssuance` and will be distributed when the contract is unpaused (or in the interim via `distributePendingIssuance()`) according to their configured proportions at the time of distribution.
38-
- **Self-minting continues**: Self-minting targets can still query their allocation, but should check the `blockAppliedTo` fields to respect pause state. Because RewardsManager does not check `blockAppliedTo` and will mint tokens even when the allocator is paused, the initial implementation does not pause self-minting targets. (This behavior is subject to change in future versions, and new targets should not check `blockAppliedTo`.) Note that RewardsManager is indepently pausable.
38+
- **Self-minting continues**: Self-minting targets can still query their allocation, but should check the `blockAppliedTo` fields to respect pause state. Because RewardsManager does not check `blockAppliedTo` and will mint tokens even when the allocator is paused, the initial implementation does not pause self-minting targets. (This behavior is subject to change in future versions, and new targets should check `blockAppliedTo`.) Note that RewardsManager is independently pausable.
3939
- **Configuration allowed**: Governance functions like `setIssuancePerBlock()` and `setTargetAllocation()` still work. However, unlike changes made while unpaused, changes made will be applied from lastIssuanceDistributionBlock rather than the current block.
4040
- **Notifications continue**: Targets are still notified of allocation changes, and should check the `blockAppliedTo` fields to correctly apply changes.
4141

@@ -45,7 +45,8 @@ During pause periods, the contract tracks:
4545

4646
- `lastIssuanceAccumulationBlock`: Updated to current block whenever accumulation occurs
4747
- `pendingAccumulatedAllocatorIssuance`: Accumulates issuance intended for allocator-minting targets
48-
- Calculation: `(issuancePerBlock * blocksSinceLastAccumulation * totalAllocatorMintingAllocationPPM) / MILLION`
48+
- Calculation: `(issuancePerBlock * blocksSinceLastAccumulation * (MILLION - totalSelfMintingAllocationPPM)) / MILLION`
49+
- **Internal accumulation**: The contract uses private `accumulatePendingIssuance()` functions to handle accumulation logic, which can be triggered automatically during rate changes or manually via the public `distributePendingIssuance(uint256)` function
4950

5051
#### Recovery Process
5152

@@ -83,7 +84,9 @@ The contract uses ERC-7201 namespaced storage to prevent storage collisions in u
8384

8485
### Constants
8586

86-
The contract inherits the following constant from `BaseUpgradeable`.
87+
The contract inherits the following constant from `BaseUpgradeable`:
88+
89+
- **MILLION**: `1,000,000` - Used as the denominator for Parts Per Million (PPM) calculations. For example, 50% allocation would be represented as 500,000 PPM.
8790

8891
## Core Functions
8992

@@ -189,6 +192,19 @@ The contract provides multiple overloaded functions for setting target allocatio
189192
- Can be called even when the contract is paused
190193
- No-op if there is no pending issuance or all targets are self-minting
191194

195+
#### `distributePendingIssuance(uint256 toBlockNumber) → uint256`
196+
197+
- **Access**: GOVERNOR_ROLE only
198+
- **Purpose**: Accumulate pending issuance up to a specific block, then distribute all accumulated issuance
199+
- **Parameters**:
200+
- `toBlockNumber` - Block number to accumulate to (must be >= lastIssuanceAccumulationBlock and <= current block)
201+
- **Returns**: Block number up to which issuance has been distributed
202+
- **Notes**:
203+
- First accumulates pending issuance up to the specified block
204+
- Then distributes all accumulated issuance to allocator-minting targets
205+
- Can be called even when the contract is paused
206+
- Will revert with `ToBlockOutOfRange()` if toBlockNumber is invalid
207+
192208
### View Functions
193209

194210
#### `getTargetAllocation(address target) → Allocation`
@@ -305,9 +321,6 @@ Before any allocation changes, targets are notified via the `IIssuanceTarget.bef
305321
- Failed notifications cause the entire transaction to revert
306322
- Use `forceTargetNoChangeNotificationBlock()` to skip notification for broken targets before removing them
307323
- Notifications cannot be skipped (the `evenIfDistributionPending` parameter only affects distribution requirements)
308-
- Failed notifications cause the entire transaction to revert
309-
- Use `forceNoChangeNotificationBlock()` to skip notification for malfunctioning targets before removing them
310-
- Notifications cannot be skipped (the `force` parameter only affects distribution requirements)
311324
- Manual notification is available for gas limit recovery via `notifyTarget()`
312325

313326
## Gas Limit Recovery
@@ -340,11 +353,19 @@ event IssuancePerBlockUpdated(uint256 oldIssuancePerBlock, uint256 newIssuancePe
340353
## Error Conditions
341354

342355
```solidity
343-
error IssuanceAllocatorTargetAddressCannotBeZero();
344-
error IssuanceAllocatorInsufficientAllocationAvailable();
345-
error IssuanceAllocatorTargetDoesNotSupportIIssuanceTarget();
356+
error TargetAddressCannotBeZero();
357+
error InsufficientAllocationAvailable();
358+
error TargetDoesNotSupportIIssuanceTarget();
359+
error ToBlockOutOfRange();
346360
```
347361

362+
### Error Descriptions
363+
364+
- **TargetAddressCannotBeZero**: Thrown when attempting to set allocation for the zero address
365+
- **InsufficientAllocationAvailable**: Thrown when the total allocation would exceed 1,000,000 PPM (100%)
366+
- **TargetDoesNotSupportIIssuanceTarget**: Thrown when a target contract does not implement the required IIssuanceTarget interface
367+
- **ToBlockOutOfRange**: Thrown when the `toBlockNumber` parameter in `distributePendingIssuance(uint256)` is outside the valid range (must be >= lastIssuanceAccumulationBlock and <= current block)
368+
348369
## Usage Patterns
349370

350371
### Initial Setup

0 commit comments

Comments
 (0)