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.

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

Jump to bottom

fix: not pledged #47

Merged
merged 9 commits into from
Jul 24, 2024
Merged

fix: not pledged #47

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
55afd31
fix: adding escalated status
ashitakah Jul 8, 2024
82a20f9
fix: issue anyone can steal pledge
ashitakah Jul 8, 2024
7325fcd
Merge branch 'dev' into feat/not-pledged
0xShaito Jul 9, 2024
f8502c6
Merge branch 'dev' into feat/not-pledged
ashitakah Jul 13, 2024
13a3ca5
feat: merge with dev
ashitakah Jul 19, 2024
2ecae94
fix: tests
ashitakah Jul 19, 2024
b447844
feat: no resolution
ashitakah Jul 22, 2024
b4f76dc
Merge branch 'feat/not-pledged' of github.com:defi-wonderland/prophet…
ashitakah Jul 22, 2024
5161792
feat: tests
ashitakah Jul 24, 2024
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
8 changes: 7 additions & 1 deletion solidity/contracts/extensions/BondEscalationAccounting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
mapping(bytes32 _disputeId => EscalationResult _result) public escalationResults;

/// @inheritdoc IBondEscalationAccounting
mapping(bytes32 _requestId => mapping(address _pledger => bool)) public pledgerClaimed;
mapping(bytes32 _requestId => mapping(address _pledger => bool _claimed)) public pledgerClaimed;

mapping(bytes32 _disputeId => mapping(address _pledger => bool _pledged)) public pledgerHasPledged;

constructor(IOracle _oracle) AccountingExtension(_oracle) {}

Expand All @@ -33,6 +35,8 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount

pledges[_disputeId][_token] += _amount;

pledgerHasPledged[_disputeId][_pledger] = true;
Copy link
Member

Choose a reason for hiding this comment

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

Can't the same pledger pledge multiple times? This would kill that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, but as far I know if the resolution is non, the user can claim their pledge 1 time, so with that will be solve I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Missed the check for amount of pledges. You are right! Tests are failing though


unchecked {
balanceOf[_pledger][_token] -= _amount;
}
Expand Down Expand Up @@ -78,6 +82,7 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
if (_result.token == IERC20(address(0))) revert BondEscalationAccounting_NoEscalationResult();
bytes32 _requestId = _result.requestId;
if (pledgerClaimed[_requestId][_pledger]) revert BondEscalationAccounting_AlreadyClaimed();
if (!pledgerHasPledged[_disputeId][_pledger]) revert BondEscalationAccounting_NotPledged();

IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId);
uint256 _amountPerPledger = _result.amountPerPledger;
Expand All @@ -95,6 +100,7 @@ contract BondEscalationAccounting is AccountingExtension, IBondEscalationAccount
uint256 _claimAmount = _amountPerPledger * _numberOfPledges;

pledgerClaimed[_requestId][_pledger] = true;
pledgerHasPledged[_disputeId][_pledger] = false;
balanceOf[_pledger][_token] += _claimAmount;

unchecked {
Expand Down
10 changes: 8 additions & 2 deletions solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
function commitVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute, bytes32 _commitment) public {
bytes32 _disputeId = _getId(_dispute);
if (ORACLE.createdAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.None) {
if (
ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.None
&& ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated
) {
revert PrivateERC20ResolutionModule_AlreadyResolved();
}

Expand Down Expand Up @@ -114,7 +117,10 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule {
IOracle.Dispute calldata _dispute
) external onlyOracle {
if (ORACLE.createdAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute();
if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.None) {
if (
ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.None
&& ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated
) {
revert PrivateERC20ResolutionModule_AlreadyResolved();
}

Expand Down
14 changes: 14 additions & 0 deletions solidity/interfaces/extensions/IBondEscalationAccounting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ interface IBondEscalationAccounting is IAccountingExtension {
*/
error BondEscalationAccounting_AlreadySettled();

/**
* @notice Thrown when an user tries to claim their pledge for an escalation which wasn't pledged
*/
error BondEscalationAccounting_NotPledged();

/*///////////////////////////////////////////////////////////////
STRUCTS
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -160,6 +165,15 @@ interface IBondEscalationAccounting is IAccountingExtension {
*/
function pledgerClaimed(bytes32 _requestId, address _pledger) external returns (bool _claimed);

/**
* @notice True if the given pledger has pledged for the given dispute
*
* @param _disputeId The ID of the bond-escalated dispute
* @param _pledger Address of the pledger
* @return _pledged True if the pledger has pledged
*/
function pledgerHasPledged(bytes32 _disputeId, address _pledger) external returns (bool _pledged);

/*///////////////////////////////////////////////////////////////
LOGIC
//////////////////////////////////////////////////////////////*/
Expand Down
18 changes: 18 additions & 0 deletions solidity/test/unit/modules/dispute/BondEscalationAccounting.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import {Strings} from '@openzeppelin/contracts/utils/Strings.sol';
contract ForTest_BondEscalationAccounting is BondEscalationAccounting {
constructor(IOracle _oracle) BondEscalationAccounting(_oracle) {}

function forTest_setPledgerHasPledged(bytes32 _disputeId, address _pledger, bool _pledged) public {
pledgerHasPledged[_disputeId][_pledger] = _pledged;
}

function forTest_setPledge(bytes32 _disputeId, IERC20 _token, uint256 _amount) public {
pledges[_disputeId][_token] = _amount;
}
Expand Down Expand Up @@ -445,6 +449,16 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {
bondEscalationAccounting.claimEscalationReward(_disputeId, pledger);
}

function test_revertIfNotPledged(bytes32 _disputeId, bytes32 _requestId) public {
bondEscalationAccounting.forTest_setEscalationResult(
_disputeId, _requestId, token, 0, IBondEscalationModule(address(this))
);

vm.expectRevert(IBondEscalationAccounting.BondEscalationAccounting_NotPledged.selector);

bondEscalationAccounting.claimEscalationReward(_disputeId, pledger);
}

function test_forVotesWon(
bytes32 _disputeId,
bytes32 _requestId,
Expand All @@ -462,6 +476,8 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {

bondEscalationAccounting.forTest_setPledge(_disputeId, token, _amount * _pledges);

bondEscalationAccounting.forTest_setPledgerHasPledged(_disputeId, pledger, true);
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a test for someone who pledged multiple times?


// Mock and expect to call the oracle getting the dispute status
_mockAndExpect(
address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Won)
Expand Down Expand Up @@ -507,6 +523,8 @@ contract BondEscalationAccounting_Unit_ClaimEscalationReward is BaseTest {

bondEscalationAccounting.forTest_setPledge(_disputeId, token, _amount * _pledges);

bondEscalationAccounting.forTest_setPledgerHasPledged(_disputeId, pledger, true);

// Mock and expect to call the oracle getting the dispute status
_mockAndExpect(
address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Lost)
Expand Down
Loading