Skip to content
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.

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

Deacrease power vuln #510

Merged
merged 22 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b726c2a
initial fix for decreasePower problem
kafann Dec 20, 2024
e5ba17f
Added calculation for support to remove in each proposal
kafann Dec 20, 2024
053a5e2
added tests, need to fix strings.sol error cant run test
kafann Jan 4, 2025
67cf0be
Finished decrease logic to remove supported proposals, wrote 2 tests …
kafann Jan 5, 2025
85c6379
Finsihed testing all cases, ready for merge and test on ui side
kafann Jan 9, 2025
5f7c8bb
Add test 100 50 75
Corantin Jan 10, 2025
dd331bf
Completed feature + fixed edge case
kafann Jan 19, 2025
58c684f
Merge remote-tracking branch 'origin/dev' into deacrease-power-vuln
Corantin Jan 26, 2025
734700d
Merge remote-tracking branch 'origin/deacrease-power-vuln' into deacr…
Corantin Jan 26, 2025
c4247bf
Remove the if unusedPower equals zero + Add new test for it
Corantin Jan 26, 2025
88ff29e
Reduce contract size
Corantin Jan 26, 2025
8d95a29
Remove error CantDecreaseWhileSupporting();
Corantin Jan 26, 2025
5671637
Contract fix SupportAdded not being passed the staked point + Deploy …
Corantin Jan 26, 2025
2f63090
Use published subgraph for breadcrumbs in priority
Corantin Jan 26, 2025
d42b87c
Merge branch 'dev' into deacrease-power-vuln
Corantin Jan 29, 2025
5c9c8dc
Fixed underflow error and pointsToUnstake edge case on Capped point s…
kafann Feb 2, 2025
1dc2265
Removed extra else (useless because pointsToDcrease already 0
kafann Feb 2, 2025
331192e
New upgrade for arbsep
Corantin Feb 5, 2025
e12ade7
removed supportsInterface
kafann Feb 8, 2025
a9ee73e
PUSH fix
Corantin Feb 9, 2025
c7cbb24
New upgrade for prod + new subgraph version 0.1.12
Corantin Feb 10, 2025
035b3ad
Merge remote-tracking branch 'origin/dev' into deacrease-power-vuln
Corantin Feb 10, 2025
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
2 changes: 1 addition & 1 deletion pkg/contracts/out/Allo.sol/Allo.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/CVStrategyV0_0.sol/CVStrategyV0_0.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/CVStrategyV0_0.sol/IPointStrategy.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/CollateralVault.sol/CollateralVault.json

Large diffs are not rendered by default.

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/contracts/out/ERC20.sol/ERC20.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/ERC20/IERC20.sol/IERC20.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/FAllo.sol/FAllo.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/GV2ERC20.sol/GV2ERC20.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IAllo.sol/IAllo.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IArbitrator.sol/IArbitrator.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IERC20.sol/IERC20.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IERC20Metadata.sol/IERC20Metadata.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IERC20Permit.sol/IERC20Permit.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/IRegistry.sol/IRegistry.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/ISafe.sol/Enum.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/ISafe.sol/ISafe.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/ISafe.sol/SafeProxyFactory.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/MockERC20.sol/MockERC20.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/PassportScorer.sol/PassportScorer.json

Large diffs are not rendered by default.

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/contracts/out/Registry.sol/Registry.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/contracts/out/RegistrySetup.sol/RegistrySetup.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/contracts/out/RegistrySetup.sol/RegistrySetupFull.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/SafeArbitrator.sol/SafeArbitrator.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/contracts/out/SafeERC20.sol/SafeERC20.json

Large diffs are not rendered by default.

This file was deleted.

1 change: 0 additions & 1 deletion pkg/contracts/out/TERC20.sol/TERC20.json

This file was deleted.

This file was deleted.

75 changes: 49 additions & 26 deletions pkg/contracts/src/CVStrategy/CVStrategyV0_0.sol
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
// error RegistryCannotBeZero(); // 0x5df4b1ef
// error SupportUnderflow(uint256 _support, int256 _delta, int256 _result); // 0x3bbc7142
error NotEnoughPointsToSupport(uint256 pointsSupport, uint256 pointsBalance); // 0xd64182fe
error CantDecreaseWhileSupporting();
Corantin marked this conversation as resolved.
Show resolved Hide resolved

// error ProposalDataIsEmpty(); //0xc5f7c4c0
// error ProposalIdCannotBeZero(); //0xf881a10d
Expand Down Expand Up @@ -510,13 +511,32 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
function decreasePower(address _member, uint256 _amountToUnstake) external virtual returns (uint256) {
onlyRegistryCommunity();
//requireMemberActivatedInStrategies

uint256 pointsToDecrease = 0;
if (pointSystem == PointSystem.Unlimited || pointSystem == PointSystem.Capped) {
pointsToDecrease = _amountToUnstake; // from decreasePowerCappedUnlimited(_amountToUnstake)
} else {
pointsToDecrease = decreasePowerQuadratic(_member, _amountToUnstake);
}
uint256 voterStake = totalVoterStakePct[_member];
uint256 unusedPower = registryCommunity.getMemberPowerInStrategy(_member, address(this)) - voterStake;
if (unusedPower < pointsToDecrease) {
uint256 removedRatio = (pointsToDecrease << 128) / voterStake;
for(uint256 i=0; i < voterStakedProposals[_member].length; i++) {
uint256 proposalId = voterStakedProposals[_member][i];
Proposal storage proposal = proposals[proposalId];
uint256 stakedPoints = proposal.voterStakedPoints[_member];
//This calculation is right ?
uint256 newStakedPoints = stakedPoints - ((stakedPoints * removedRatio + (1 << 127)) >> 128);
uint256 oldStake = proposal.stakedAmount;
proposal.stakedAmount -= stakedPoints - newStakedPoints;
proposal.voterStakedPoints[_member] = newStakedPoints;
totalStaked -= stakedPoints - newStakedPoints;
totalVoterStakePct[_member] -= stakedPoints - newStakedPoints;
//Here we pass oldStake??
_calculateAndSetConviction(proposal, oldStake);
emit SupportAdded(_member, proposalId, 0, proposal.stakedAmount, proposal.convictionLast);
}
}
totalPointsActivated -= pointsToDecrease;
emit PowerDecreased(_member, _amountToUnstake, pointsToDecrease);
return pointsToDecrease;
Expand Down Expand Up @@ -815,9 +835,9 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
}

// Goss: Commented because accessible through public fields
// function getProposalStakedAmount(uint256 _proposalId) external view virtual returns (uint256) {
// return proposals[_proposalId].stakedAmount;
// }
function getProposalStakedAmount(uint256 _proposalId) external view virtual returns (uint256) {
return proposals[_proposalId].stakedAmount;
}
// do a internal function to get the total voter stake

// Goss: Commented because accessible through public fields
Expand All @@ -826,28 +846,28 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
// }

// Goss: Commented because accessible through public fields
// function getArbitrableConfig()
// external
// view
// virtual
// returns (
// IArbitrator arbitrator,
// address tribunalSafe,
// uint256 submitterCollateralAmount,
// uint256 challengerCollateralAmount,
// uint256 defaultRuling,
// uint256 defaultRulingTimeout
// )
// {
// return (
// arbitrableConfigs[currentArbitrableConfigVersion].arbitrator,
// arbitrableConfigs[currentArbitrableConfigVersion].tribunalSafe,
// arbitrableConfigs[currentArbitrableConfigVersion].submitterCollateralAmount,
// arbitrableConfigs[currentArbitrableConfigVersion].challengerCollateralAmount,
// arbitrableConfigs[currentArbitrableConfigVersion].defaultRuling,
// arbitrableConfigs[currentArbitrableConfigVersion].defaultRulingTimeout
// );
// }
function getArbitrableConfig()
external
view
virtual
returns (
IArbitrator arbitrator,
address tribunalSafe,
uint256 submitterCollateralAmount,
uint256 challengerCollateralAmount,
uint256 defaultRuling,
uint256 defaultRulingTimeout
)
{
return (
arbitrableConfigs[currentArbitrableConfigVersion].arbitrator,
arbitrableConfigs[currentArbitrableConfigVersion].tribunalSafe,
arbitrableConfigs[currentArbitrableConfigVersion].submitterCollateralAmount,
arbitrableConfigs[currentArbitrableConfigVersion].challengerCollateralAmount,
arbitrableConfigs[currentArbitrableConfigVersion].defaultRuling,
arbitrableConfigs[currentArbitrableConfigVersion].defaultRulingTimeout
);
}

function _internal_getProposalVoterStake(uint256 _proposalId, address _voter)
internal
Expand Down Expand Up @@ -946,6 +966,8 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
// console.log("stakedPointsPct%", stakedPointsPct);

proposal.voterStakedPoints[_sender] = stakedPoints;
console.log("proposal.voterStakedPoints[_sender]", proposal.voterStakedPoints[_sender]);
Corantin marked this conversation as resolved.
Show resolved Hide resolved


// console.log("_sender", _sender);
// uint2stakedPointsunt = stakedPoints;
Expand Down Expand Up @@ -977,6 +999,7 @@ contract CVStrategyV0_0 is BaseStrategyUpgradeable, IArbitrable, IPointStrategy,
_calculateAndSetConviction(proposal, previousStakedPoints);
emit SupportAdded(_sender, proposalId, stakedPoints, proposal.stakedAmount, proposal.convictionLast);
}
console.log("proposal.stakedAmount", proposal.stakedAmount);
Corantin marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Loading