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 refund calculation bug in BetaProgramInitiator #228

Merged
merged 5 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion contracts/contracts/coordination/BetaProgramInitiator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ contract BetaProgramInitiator {
);

// Process pending fees in Coordinator, if necessary
uint256 refundAmount = coordinator.feeDeduction(request.payment, request.duration);
uint256 refundAmount = request.payment;
refundAmount -= coordinator.feeDeduction(request.payment, request.duration);
if (coordinator.pendingFees(ritualId) > 0) {
coordinator.processPendingFee(ritualId);
}
Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/coordination/FlatRateFeeModel.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract FlatRateFeeModel is IFeeModel {
}

// TODO: Validate if this is enough to remove griefing attacks
function feeDeduction(uint256 pending, uint256 duration) public pure returns (uint256) {
return (pending * 1 days) / duration;
function feeDeduction(uint256 pending, uint256) public pure returns (uint256) {
return pending;
Copy link
Member

Choose a reason for hiding this comment

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

Should we file a follow-up issue for a post-beta update to this value i.e. to actually allow refunds.

Copy link
Member Author

Choose a reason for hiding this comment

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

opened #229

}
}
2 changes: 1 addition & 1 deletion contracts/test/BetaProgramInitiatorTestSet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ contract CoordinatorForBetaProgramInitiatorMock {
}

function feeDeduction(uint256 pending, uint256) public pure returns (uint256) {
return pending / 2;
return pending / 10;
}

function pendingFees(uint256 _ritualId) external view returns (uint256) {
Expand Down
47 changes: 37 additions & 10 deletions tests/test_beta_program_initiator.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ def test_refund(accounts, beta_program_initiator, token, coordinator, executor):
nodes = [node_1, node_2]
duration_1 = DAY_IN_SECONDS
ritual_cost_1 = coordinator.getRitualInitiationCost(nodes, duration_1)
duration_2 = 2 * duration_1
duration_2 = 3 * duration_1
ritual_cost_2 = coordinator.getRitualInitiationCost(nodes, duration_2)

token.mint(initiator_1, 10 * ritual_cost_1, sender=initiator_1)
Expand Down Expand Up @@ -393,33 +393,60 @@ def test_refund(accounts, beta_program_initiator, token, coordinator, executor):

# Refund failed request
coordinator.setRitualState(request_0_ritual_id, RitualState.DKG_TIMEOUT, sender=initiator_2)
balance_before = token.balanceOf(initiator_1)
assert coordinator.pendingFees(request_0_ritual_id) == ritual_cost_1

assert token.balanceOf(beta_program_initiator.address) == 0
initiator_1_balance_before = token.balanceOf(initiator_1)
coordinator_balance_before = token.balanceOf(coordinator.address)
assert coordinator_balance_before == ritual_cost_1 + ritual_cost_2
pending_fees_1_before = coordinator.pendingFees(request_0_ritual_id)
assert pending_fees_1_before == ritual_cost_1

tx = beta_program_initiator.refundFailedRequest(0, sender=initiator_2)

coordinator_balance_after = token.balanceOf(coordinator.address)
fee_deduction_1 = coordinator.feeDeduction(pending_fees_1_before, duration_1)
pending_fees_1_after = coordinator.pendingFees(request_0_ritual_id)
assert coordinator_balance_after == ritual_cost_2 + fee_deduction_1
assert pending_fees_1_after == 0
assert token.balanceOf(beta_program_initiator.address) == 0
balance_after = token.balanceOf(initiator_1)
assert balance_after - balance_before == ritual_cost_1 // 2

refund_1 = ritual_cost_1 - fee_deduction_1
initiator_1_balance_after = token.balanceOf(initiator_1)
assert initiator_1_balance_after - initiator_1_balance_before == refund_1
assert beta_program_initiator.requests(0)[RITUAL_ID_SLOT] == request_0_ritual_id
assert beta_program_initiator.requests(0)[PAYMENT_SLOT] == 0

events = beta_program_initiator.FailedRequestRefunded.from_receipt(tx)
assert events == [beta_program_initiator.FailedRequestRefunded(0, ritual_cost_1 // 2)]
assert events == [beta_program_initiator.FailedRequestRefunded(0, refund_1)]

# Can't refund again
with ape.reverts("Refund already processed"):
beta_program_initiator.refundFailedRequest(0, sender=executor)

# Refund failed request without pending fees
coordinator.setRitualState(request_1_ritual_id, RitualState.DKG_INVALID, sender=initiator_2)
balance_before = token.balanceOf(initiator_2)

assert token.balanceOf(beta_program_initiator.address) == 0
initiator_2_balance_before = token.balanceOf(initiator_2)
coordinator_balance_before = token.balanceOf(coordinator.address)
assert coordinator_balance_before == ritual_cost_2 + fee_deduction_1
pending_fees_2_before = coordinator.pendingFees(request_1_ritual_id)
assert pending_fees_2_before == ritual_cost_2

coordinator.processPendingFee(request_1_ritual_id, sender=initiator_1)
assert coordinator.pendingFees(request_1_ritual_id) == 0

fee_deduction_2 = coordinator.feeDeduction(pending_fees_2_before, duration_2)
refund_2 = ritual_cost_2 - fee_deduction_2
assert token.balanceOf(beta_program_initiator.address) == refund_2

tx = beta_program_initiator.refundFailedRequest(1, sender=initiator_2)

assert token.balanceOf(beta_program_initiator.address) == 0
balance_after = token.balanceOf(initiator_2)
assert balance_after - balance_before == ritual_cost_2 // 2
initiator_2_balance_after = token.balanceOf(initiator_2)
assert initiator_2_balance_after - initiator_2_balance_before == refund_2
assert beta_program_initiator.requests(1)[RITUAL_ID_SLOT] == request_1_ritual_id
assert beta_program_initiator.requests(1)[PAYMENT_SLOT] == 0

events = beta_program_initiator.FailedRequestRefunded.from_receipt(tx)
assert events == [beta_program_initiator.FailedRequestRefunded(1, ritual_cost_2 // 2)]
assert events == [beta_program_initiator.FailedRequestRefunded(1, refund_2)]
5 changes: 3 additions & 2 deletions tests/test_coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ def test_post_aggregation_fails(
fee = coordinator.getRitualInitiationCost(nodes, DURATION)
assert erc20.balanceOf(coordinator) == fee
assert coordinator.totalPendingFees() == fee
assert coordinator.pendingFees(ritualID) == fee
pending_fee = coordinator.pendingFees(ritualID)
assert pending_fee == fee
with ape.reverts("Can't withdraw pending fees"):
coordinator.withdrawTokens(erc20.address, 1, sender=treasury)

Expand All @@ -428,7 +429,7 @@ def test_post_aggregation_fails(
initiator_balance_after_refund = erc20.balanceOf(initiator)
coordinator_balance_after_refund = erc20.balanceOf(coordinator)
refund = initiator_balance_after_refund - initiator_balance_before_refund
assert refund == coordinator.getRitualInitiationCost(nodes, ONE_DAY)
assert refund == fee - coordinator.feeDeduction(pending_fee, DURATION)
assert coordinator_balance_after_refund + refund == fee
assert coordinator.totalPendingFees() == 0
assert coordinator.pendingFees(ritualID) == 0
Expand Down
Loading