Skip to content

Commit 4dabb17

Browse files
committed
fix(echidna): avoid OOB currentCommits/currentReveals getter reverts
Solidity public array getters revert on out-of-bounds access (mapped to currentCommits declaration in Redistribution.sol). Echidna traces those as failures even when wrapped in staticcall. - Add RedistributionExposed.sol with currentCommitsLength/currentRevealsLength - Bound redistribution + system harness scans using real lengths (cap 25) - Simplify _scanRevealsLen to use length instead of probing past end Made-with: Cursor
1 parent a3d7586 commit 4dabb17

3 files changed

Lines changed: 63 additions & 30 deletions

File tree

src/echidna/EchidnaRedistributionHarness.sol

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pragma solidity ^0.8.19;
33

44
import "../Redistribution.sol";
55
import "../interface/IPostageStamp.sol";
6+
import "./RedistributionExposed.sol";
67

78
contract EchidnaStakeRegistryMock is IStakeRegistry {
89
struct Node {
@@ -150,18 +151,6 @@ contract EchidnaPostageStampMock is IPostageStamp {
150151
}
151152
}
152153

153-
contract RedistributionExposed is Redistribution {
154-
constructor(
155-
address staking,
156-
address postageContract,
157-
address oracleContract
158-
) Redistribution(staking, postageContract, oracleContract) {}
159-
160-
function exposedWinnerSelection() external {
161-
winnerSelection();
162-
}
163-
}
164-
165154
contract EchidnaRedistributionActor {
166155
RedistributionExposed internal immutable redist;
167156

@@ -218,6 +207,8 @@ contract EchidnaRedistributionHarness {
218207
RedistributionExposed internal immutable redist;
219208

220209
uint256 internal constant ACTOR_COUNT = 3;
210+
/// @dev Cap scans; must match pending winnerSelection snapshot arrays (size 25).
211+
uint256 internal constant MAX_COMMIT_REVEAL_SCAN = 25;
221212
EchidnaRedistributionActor[3] internal actors;
222213

223214
// Forbidden-call flags.
@@ -286,6 +277,16 @@ contract EchidnaRedistributionHarness {
286277
pendingWinnerSelectionLen = 0;
287278
}
288279

280+
function _boundedCommitsLen() internal view returns (uint256) {
281+
uint256 n = redist.currentCommitsLength();
282+
return n > MAX_COMMIT_REVEAL_SCAN ? MAX_COMMIT_REVEAL_SCAN : n;
283+
}
284+
285+
function _boundedRevealsLen() internal view returns (uint256) {
286+
uint256 n = redist.currentRevealsLength();
287+
return n > MAX_COMMIT_REVEAL_SCAN ? MAX_COMMIT_REVEAL_SCAN : n;
288+
}
289+
289290
// -----------------------------
290291
// Actions
291292
// -----------------------------
@@ -501,7 +502,8 @@ contract EchidnaRedistributionHarness {
501502
// Snapshot current commits (bounded) and freeze counts before selection.
502503
pendingWinnerSelectionRound = redist.currentRound();
503504

504-
for (uint256 i = 0; i < 25; i++) {
505+
uint256 commitLim = _boundedCommitsLen();
506+
for (uint256 i = 0; i < commitLim; i++) {
505507
(bool ok, bytes memory data) = address(redist).staticcall(
506508
abi.encodeWithSignature("currentCommits(uint256)", i)
507509
);
@@ -554,16 +556,18 @@ contract EchidnaRedistributionHarness {
554556

555557
function echidna_reveal_entries_imply_matching_commit() external view returns (bool) {
556558
// For each reveal entry, there must exist a commit marked revealed with matching overlay/owner and revealIndex pointing here.
557-
for (uint256 i = 0; i < 25; i++) {
559+
uint256 rLim = _boundedRevealsLen();
560+
uint256 cLim = _boundedCommitsLen();
561+
for (uint256 i = 0; i < rLim; i++) {
558562
(bool okR, bytes32 rOverlay, address rOwner) = _revealOverlayOwner(i);
559-
if (!okR) break;
563+
if (!okR) return false;
560564

561565
bool found = false;
562-
for (uint256 j = 0; j < 25; j++) {
566+
for (uint256 j = 0; j < cLim; j++) {
563567
(bool okC, bytes32 cOverlay, address cOwner, bool cRevealed, uint256 cRevealIndex) = _commitRevealLink(
564568
j
565569
);
566-
if (!okC) break;
570+
if (!okC) return false;
567571
if (cRevealed && cRevealIndex == i && cOverlay == rOverlay && cOwner == rOwner) {
568572
found = true;
569573
break;
@@ -685,7 +689,8 @@ contract EchidnaRedistributionHarness {
685689
address[25] memory owner
686690
)
687691
{
688-
for (uint256 i = 0; i < 25; i++) {
692+
uint256 lim = _boundedCommitsLen();
693+
for (uint256 i = 0; i < lim; i++) {
689694
(bool ok, bytes32 ov, address ow, bool rev, uint256 ri) = _commitFields(i);
690695
if (!ok) break;
691696
overlays[i] = ov;
@@ -697,11 +702,7 @@ contract EchidnaRedistributionHarness {
697702
}
698703

699704
function _scanRevealsLen() internal view returns (uint256 n) {
700-
for (uint256 i = 0; i < 25; i++) {
701-
(bool ok, , ) = _revealOverlayOwner(i);
702-
if (!ok) break;
703-
n++;
704-
}
705+
n = _boundedRevealsLen();
705706
}
706707

707708
function _commitOverlayOwner(uint256 i) internal view returns (bool ok, bytes32 ov, address ow) {
@@ -821,7 +822,8 @@ contract EchidnaRedistributionHarness {
821822
}
822823

823824
function _findCommit(bytes32 overlay, bytes32 obfuscated) internal view returns (bool ok, uint256 idx) {
824-
for (uint256 i = 0; i < 25; i++) {
825+
uint256 lim = _boundedCommitsLen();
826+
for (uint256 i = 0; i < lim; i++) {
825827
(bool okI, bytes32 ov, , , , , bytes32 obf, ) = _commitFull(i);
826828
if (!okI) break;
827829
if (ov == overlay && obf == obfuscated) return (true, i);
@@ -830,7 +832,8 @@ contract EchidnaRedistributionHarness {
830832
}
831833

832834
function _commitOverlayExists(bytes32 overlay) internal view returns (bool) {
833-
for (uint256 i = 0; i < 25; i++) {
835+
uint256 lim = _boundedCommitsLen();
836+
for (uint256 i = 0; i < lim; i++) {
834837
(bool ok, bytes32 ov, , , ) = _commitFields(i);
835838
if (!ok) break;
836839
if (ov == overlay) return true;

src/echidna/EchidnaSystemHarness.sol

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import "../PostageStamp.sol";
66
import "../PriceOracle.sol";
77
import "../Redistribution.sol" as RedistMod;
88
import "../Staking.sol" as StakingMod;
9+
import "./RedistributionExposed.sol";
910

1011
contract EchidnaSystemActor {
1112
TestToken internal immutable token;
@@ -118,8 +119,10 @@ contract EchidnaSystemHarness {
118119
// Deploy stake registry (uses oracle.currentPrice()).
119120
stake = new StakingMod.StakeRegistry(address(token), 1, address(oracle));
120121

121-
// Deploy redistribution (uses stake/stamp/oracle).
122-
redist = new RedistMod.Redistribution(address(stake), address(stamp), address(oracle));
122+
// Deploy redistribution (uses stake/stamp/oracle). Exposed wrapper adds length helpers for harness scans.
123+
redist = RedistMod.Redistribution(
124+
address(new RedistributionExposed(address(stake), address(stamp), address(oracle)))
125+
);
123126

124127
// Wire roles: redistribution must be able to freeze stake and withdraw the stamp pot.
125128
stake.grantRole(stake.REDISTRIBUTOR_ROLE(), address(redist));
@@ -353,8 +356,9 @@ contract EchidnaSystemHarness {
353356
}
354357

355358
function _commitExists(bytes32 obfuscated, address owner) internal view returns (bool) {
356-
// currentCommits is deleted each new commit round; bounded scan to avoid unbounded loops.
357-
for (uint256 i = 0; i < 25; i++) {
359+
uint256 lim = RedistributionExposed(address(redist)).currentCommitsLength();
360+
if (lim > 25) lim = 25;
361+
for (uint256 i = 0; i < lim; i++) {
358362
(bool ok, bytes memory data) = address(redist).staticcall(
359363
abi.encodeWithSignature("currentCommits(uint256)", i)
360364
);
@@ -374,7 +378,9 @@ contract EchidnaSystemHarness {
374378
}
375379

376380
function _revealMatchesCommit(address owner, uint8 depth, bytes32 hash) internal view returns (bool) {
377-
for (uint256 i = 0; i < 25; i++) {
381+
uint256 lim = RedistributionExposed(address(redist)).currentRevealsLength();
382+
if (lim > 25) lim = 25;
383+
for (uint256 i = 0; i < lim; i++) {
378384
(bool ok, bytes memory data) = address(redist).staticcall(
379385
abi.encodeWithSignature("currentReveals(uint256)", i)
380386
);
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
pragma solidity ^0.8.19;
3+
4+
import "../Redistribution.sol";
5+
6+
/// @notice Test/fuzz wrapper: exposes `winnerSelection` and array lengths so harnesses need not call
7+
/// the auto-generated `currentCommits(i)` / `currentReveals(i)` getters out of bounds (those revert).
8+
contract RedistributionExposed is Redistribution {
9+
constructor(address staking, address postageContract, address oracleContract)
10+
Redistribution(staking, postageContract, oracleContract)
11+
{}
12+
13+
function exposedWinnerSelection() external {
14+
winnerSelection();
15+
}
16+
17+
function currentCommitsLength() external view returns (uint256) {
18+
return currentCommits.length;
19+
}
20+
21+
function currentRevealsLength() external view returns (uint256) {
22+
return currentReveals.length;
23+
}
24+
}

0 commit comments

Comments
 (0)