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

fix: not pledged #47

Merged
merged 9 commits into from
Jul 24, 2024
Merged

fix: not pledged #47

merged 9 commits into from
Jul 24, 2024

Conversation

ashitakah
Copy link
Contributor

🤖 Linear

Closes GRT-8

The problem is that according to the report when the dispute is in non-resolution, everyone can access a pledge. The issue is that it doesn't check if that user made a previous pledge and anyone could subtract it in that state.

I have opted to create a mapping that both this in no resolution or if when you make a pledge you put it in true and if not in false, I do not know if it solves the problem 100%.

REVIEW!

Copy link

linear bot commented Jul 8, 2024

GRT-8 TOB-WOND-9

@@ -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

@@ -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.

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

@@ -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?

@@ -356,8 +356,8 @@ contract Integration_BondEscalation is IntegrationBase {

// Step 10: Participants claim their rewards
// Test: The requester has paid out the reward and is left with no balance
_bondEscalationAccounting.claimEscalationReward(_disputeId, requester);
assertEq(_bondEscalationAccounting.balanceOf(requester, usdc), 0, 'Mismatch: Requester balance');
// _bondEscalationAccounting.claimEscalationReward(_disputeId, requester);
Copy link
Member

Choose a reason for hiding this comment

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

Why comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mistake

_disputeId, _requestId, token, _amount, IBondEscalationModule(_bondEscalationModule)
);

bondEscalationAccounting.forTest_setPledge(_disputeId, token, _amount * _pledges * 2);
Copy link
Member

Choose a reason for hiding this comment

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

Can we have a different number of pledges for and against so we are sure we are adding both numbers correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@0xShaito 0xShaito merged commit 07e210d into dev Jul 24, 2024
3 checks passed
@0xShaito 0xShaito deleted the feat/not-pledged branch July 24, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants