Skip to content

feat: audit fixes #225

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

Merged
dDevAhmed merged 1 commit into from
May 30, 2026
Merged

feat: audit fixes #225

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
4 changes: 2 additions & 2 deletions contracts/MetaTxExample.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ contract MetaTxExample is ERC2771Context, EIP712 {
// ============ Override Functions ============

// Override to preserve original sender identity
function _msgSender() internal view override(ERC2771Context, Context) returns (address sender) {
function _msgSender() internal view override(ERC2771Context) returns (address sender) {
return ERC2771Context._msgSender();
}

function _msgData() internal view override(ERC2771Context, Context) returns (bytes calldata) {
function _msgData() internal view override(ERC2771Context) returns (bytes calldata) {
return ERC2771Context._msgData();
}

Expand Down
20 changes: 17 additions & 3 deletions contracts/ReputationSnapshot.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ contract ReputationSnapshot is AccessControl, Pausable {
/// @notice Snapshot validity window — 7 days
uint256 public constant SNAPSHOT_TTL = 7 days;

/// @notice Hard cap on users per snapshot to bound gas
uint256 public constant MAX_SNAPSHOT_SIZE = 1_000;
/// @notice Absolute hard cap on users per snapshot
uint256 public constant ABSOLUTE_MAX_SNAPSHOT_SIZE = 1_000;

/// @notice Configurable cap on users per snapshot (defaults to 200)
uint256 public maxSnapshotSize = 200;

// ----------------------------------------------------------------
// Storage
Expand Down Expand Up @@ -105,6 +108,7 @@ contract ReputationSnapshot is AccessControl, Pausable {
error ZeroAddress();
error DuplicateUser(address user);
error AlreadyFinalized(uint256 snapshotId);
error InvalidMaxSnapshotSize(uint256 provided, uint256 absoluteMax);

// ----------------------------------------------------------------
// Constructor
Expand All @@ -125,6 +129,16 @@ contract ReputationSnapshot is AccessControl, Pausable {
function pause() external onlyRole(PAUSER_ROLE) { _pause(); }
function unpause() external onlyRole(PAUSER_ROLE) { _unpause(); }

/**
* @notice Update the maximum snapshot size to bound gas in createSnapshot
* @param newMax New maximum (must be > 0 and <= ABSOLUTE_MAX_SNAPSHOT_SIZE)
*/
function setMaxSnapshotSize(uint256 newMax) external onlyRole(DEFAULT_ADMIN_ROLE) {
if (newMax == 0 || newMax > ABSOLUTE_MAX_SNAPSHOT_SIZE)
revert InvalidMaxSnapshotSize(newMax, ABSOLUTE_MAX_SNAPSHOT_SIZE);
maxSnapshotSize = newMax;
}

// ----------------------------------------------------------------
// Snapshot Creation
// ----------------------------------------------------------------
Expand All @@ -147,7 +161,7 @@ contract ReputationSnapshot is AccessControl, Pausable {
uint256 length = users.length;

if (length == 0) revert EmptySnapshot();
if (length > MAX_SNAPSHOT_SIZE) revert SnapshotTooLarge(length, MAX_SNAPSHOT_SIZE);
if (length > maxSnapshotSize) revert SnapshotTooLarge(length, maxSnapshotSize);

// Assign ID
snapshotId = ++_snapshotCounter;
Expand Down
97 changes: 97 additions & 0 deletions contracts/VerifierSlashing.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
bytes32 public constant RESOLVER_ROLE = keccak256("RESOLVER_ROLE");
bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
bytes32 public constant CRITICAL_SLASHER_ROLE = keccak256("CRITICAL_SLASHER_ROLE");

// Legacy mapping for backward compatibility
bytes32 public constant SETTLEMENT_ROLE = RESOLVER_ROLE;
Expand Down Expand Up @@ -58,6 +59,9 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc

// Verifier address => last slash timestamp
mapping(address => uint256) public lastSlashTime;

// Verifier address => last slash block number (prevents same-block bypass)
mapping(address => uint256) public lastSlashBlock;

// Total amount slashed per verifier
mapping(address => uint256) public totalSlashed;
Expand All @@ -78,6 +82,13 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
);

event StakingContractUpdated(address newStakingContract);
event CriticalSlashed(
address indexed verifier,
uint256 amount,
uint256 percentage,
string reason,
address indexed slashedBy
);

// Custom errors for gas efficiency
error UnauthorizedSlashing();
Expand All @@ -86,6 +97,8 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
error SlashingTooFrequent();
error InvalidStakingContract();
error SlashAmountTooHigh();
error SlashSameBlock();
error CriticalSlashUnauthorized();

/**
* @dev Constructor sets up roles and initial configuration
Expand Down Expand Up @@ -141,6 +154,11 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
if (block.timestamp < lastSlashTime[verifier] + slashCooldown) {
revert SlashingTooFrequent();
}

// Prevent multiple slashes in the same block (#185)
if (lastSlashBlock[verifier] == block.number) {
revert SlashSameBlock();
}

// Get current stake from staking contract
(uint256 currentStake,) = stakingContract.stakes(verifier);
Expand All @@ -158,6 +176,7 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc

// Update tracking
lastSlashTime[verifier] = block.timestamp;
lastSlashBlock[verifier] = block.number;
totalSlashed[verifier] += slashAmount;

// Record slash history
Expand Down Expand Up @@ -216,6 +235,62 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
}
}
}

/**
* @dev Critical slash — allows up to 100% slash for critical failures (#186)
* Bypasses maxSlashPercentage but still respects cooldown and block checks.
* @param verifier Address of the verifier to slash
* @param percentage Percentage 1-100 (up to MAX_SLASH_PERCENTAGE constant)
* @param reason Human-readable reason for critical slashing
*/
function criticalSlash(
address verifier,
uint256 percentage,
string calldata reason
) external nonReentrant whenNotPaused {
if (!hasRole(CRITICAL_SLASHER_ROLE, msg.sender)) {
revert CriticalSlashUnauthorized();
}

if (percentage == 0 || percentage > MAX_SLASH_PERCENTAGE) {
revert InvalidPercentage();
}

if (verifier == address(0)) {
revert NoStakeToSlash();
}

// Same-block protection still applies
if (lastSlashBlock[verifier] == block.number) {
revert SlashSameBlock();
}

(uint256 currentStake,) = stakingContract.stakes(verifier);
if (currentStake == 0) {
revert NoStakeToSlash();
}

uint256 slashAmount = (currentStake * percentage) / 100;
if (slashAmount == 0) {
revert SlashAmountTooHigh();
}

lastSlashTime[verifier] = block.timestamp;
lastSlashBlock[verifier] = block.number;
totalSlashed[verifier] += slashAmount;

slashHistory[verifier].push(SlashRecord({
timestamp: block.timestamp,
amount: slashAmount,
percentage: percentage,
reason: reason,
slashedBy: msg.sender
}));

stakingContract.forceSlash(verifier, slashAmount);

emit CriticalSlashed(verifier, slashAmount, percentage, reason, msg.sender);
}

/**
* @dev Internal slash function for batch operations
Expand All @@ -237,6 +312,11 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
if (block.timestamp < lastSlashTime[verifier] + slashCooldown) {
revert SlashingTooFrequent();
}

// Prevent multiple slashes in the same block (#185)
if (lastSlashBlock[verifier] == block.number) {
revert SlashSameBlock();
}

(uint256 currentStake,) = stakingContract.stakes(verifier);

Expand All @@ -252,6 +332,7 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc

// Update tracking
lastSlashTime[verifier] = block.timestamp;
lastSlashBlock[verifier] = block.number;
totalSlashed[verifier] += slashAmount;

slashHistory[verifier].push(SlashRecord({
Expand Down Expand Up @@ -382,6 +463,22 @@ contract VerifierSlashing is AccessControl, ReentrancyGuard, Pausable, Governanc
function revokeSettlementRole(address account) external onlyGovernanceOrAdmin {
_revokeRole(SETTLEMENT_ROLE, account);
}

/**
* @dev Grant critical slasher role (can slash up to 100%)
* @param account Address to grant the role to
*/
function grantCriticalSlasherRole(address account) external onlyGovernanceOrAdmin {
_grantRole(CRITICAL_SLASHER_ROLE, account);
}

/**
* @dev Revoke critical slasher role
* @param account Address to revoke the role from
*/
function revokeCriticalSlasherRole(address account) external onlyGovernanceOrAdmin {
_revokeRole(CRITICAL_SLASHER_ROLE, account);
}

// ============ Governance Parameter Updates ============

Expand Down
41 changes: 36 additions & 5 deletions contracts/WeightedStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {

/// @notice Whether to use weighted staking (can be disabled in emergencies)
bool public weightedStakingEnabled = true;

/// @notice Whether to apply sqrt scaling to reputation (matches API behaviour)
bool public useSqrtWeighting = true;

// Governance parameter IDs
bytes32 public constant GOVERNANCE_PARAM_MIN_REP = keccak256("MIN_REPUTATION_SCORE");
Expand All @@ -66,6 +69,7 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {
event ReputationBoundsUpdated(uint256 minScore, uint256 maxScore);
event DefaultReputationUpdated(uint256 newDefault);
event WeightedStakingToggled(bool enabled);
event SqrtWeightingToggled(bool enabled);
event WeightedStakeCalculated(
address indexed user,
uint256 rawStake,
Expand Down Expand Up @@ -135,12 +139,14 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {

// Apply bounds to reputation score
uint256 boundedScore = _applyReputationBounds(rawReputationScore);

// Apply sqrt scaling if enabled (matches API behaviour)
uint256 weight = useSqrtWeighting ? _sqrt(boundedScore * BASE_MULTIPLIER) : boundedScore;
result.reputationScore = boundedScore;
result.weight = boundedScore;
result.weight = weight;

// Calculate effective stake: stake × (reputation / BASE_MULTIPLIER)
// Using mul-div pattern to prevent overflow
result.effectiveStake = (stakeAmount * boundedScore) / BASE_MULTIPLIER;
// Calculate effective stake: stake × (weight / BASE_MULTIPLIER)
result.effectiveStake = (stakeAmount * weight) / BASE_MULTIPLIER;

return result;
}
Expand Down Expand Up @@ -233,6 +239,21 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {
return score;
}

/**
* @notice Babylonian sqrt for 18-decimal fixed-point numbers
* @param x The value to take the square root of (18 decimals)
* @return y The square root result (18 decimals)
*/
function _sqrt(uint256 x) internal pure returns (uint256 y) {
if (x == 0) return 0;
y = x;
uint256 z = (x + 1) / 2;
while (z < y) {
y = z;
z = (x / z + z) / 2;
}
}

// ============ Admin Functions ============

/**
Expand Down Expand Up @@ -284,6 +305,15 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {
emit WeightedStakingToggled(_enabled);
}

/**
* @notice Enable or disable sqrt-based weighting
* @param _enabled True to use sqrt, false for linear
*/
function setSqrtWeighting(bool _enabled) external onlyGovernanceOrAdmin {
useSqrtWeighting = _enabled;
emit SqrtWeightingToggled(_enabled);
}

// ============ Governance Parameter Updates ============

/**
Expand Down Expand Up @@ -362,6 +392,7 @@ contract WeightedStaking is AccessControl, ReentrancyGuard, GovernanceOwnable {
}

uint256 rawScore = _getReputationScore(user);
return _applyReputationBounds(rawScore);
uint256 bounded = _applyReputationBounds(rawScore);
return useSqrtWeighting ? _sqrt(bounded * BASE_MULTIPLIER) : bounded;
}
}
17 changes: 16 additions & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"@types/mocha": "^10.0.10",
"@types/node": "^20.0.0",
"chai": "^4.5.0",
"hardhat": "^2.22.17",
"hardhat": "^2.28.6",
"hardhat-gas-reporter": "^1.0.8",
"solidity-coverage": "^0.8.17",
"ts-node": "^10.9.2",
Expand Down
Loading
Loading