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

fix: account for rounding issue, and account for AMM in checkbook call #11520

Merged
merged 3 commits into from
Aug 5, 2024
Merged

fix: account for rounding issue, and account for AMM in checkbook call #11520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3b92b9
fix: account for rounding issue, and account for AMM in checkbook call
EVODelavega Aug 5, 2024
bf1c5bd
test: fix checkbook logic
EVODelavega Aug 5, 2024
b88e564
test: add multiple vAMM base scenario
EVODelavega Aug 5, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

- [11508](https://github.com/vegaprotocol/vega/issues/11508) - Handle market order for margin spam protection.
- [11507](https://github.com/vegaprotocol/vega/issues/11507) - Margin spam check to remove division by asset quantum.
- [11504](https://github.com/vegaprotocol/vega/issues/11504) - Fix bug causing panic when accounting for volume rounding with AMM orders.


## 0.77.3
Expand Down
16 changes: 14 additions & 2 deletions core/execution/amm/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,15 +427,27 @@ func (e *Engine) submit(active []*Pool, agg *types.Order, inner, outer *num.Uint

// if the pools consume the whole incoming order's volume, share it out pro-rata
if agg.Remaining < total {
maxVolumes := make([]uint64, 0, len(volumes))
// copy the available volumes for rounding.
maxVolumes = append(maxVolumes, volumes...)
var retotal uint64
for i := range volumes {
volumes[i] = agg.Remaining * volumes[i] / total
retotal += volumes[i]
}

// any lost crumbs due to integer division is given to the first pool
// any lost crumbs due to integer division is given to the pools that can accommodate it.
if d := agg.Remaining - retotal; d != 0 {
volumes[0] += d
for i, v := range volumes {
if delta := maxVolumes[i] - v; delta != 0 {
if delta >= d {
volumes[i] += d
break
}
volumes[i] += delta
d -= delta
}
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions core/integration/features/amm/0090-VAMM-001.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ Feature: Test vAMM submission works as expected
| 100 | TRADING_MODE_CONTINUOUS | 3600 | 94 | 106 | 39 | 1000 | 1 | 100 | 100 | 100 |
When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | upper bound | lower leverage | upper leverage | proposed fee |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 150 | 0.25 | 0.25 | 0.01 |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 150 | 0.25 | 0.25 | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | upper bound | lower leverage | upper leverage |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 150 | 0.25 | 0.25 |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 150 | 0.25 | 0.25 |

And set the following AMM sub account aliases:
| party | market id | alias |
| vamm1 | ETH/MAR22 | vamm1-acc |
And the following transfers should happen:
| from | from account | to | to account | market id | amount | asset | is amm | type |
| from | from account | to | to account | market id | amount | asset | is amm | type |
| vamm1 | ACCOUNT_TYPE_GENERAL | vamm1-acc | ACCOUNT_TYPE_GENERAL | | 100000 | USD | true | TRANSFER_TYPE_AMM_LOW |
6 changes: 3 additions & 3 deletions core/integration/features/amm/0090-VAMM-002.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ Feature: Test vAMM submission works as expected
| 100 | TRADING_MODE_CONTINUOUS | 3600 | 94 | 106 | 39 | 1000 | 1 | 100 | 100 | 100 |
When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | proposed fee |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 0.25 | 0.01 |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 0.25 | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 0.25 |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 0.25 |

And set the following AMM sub account aliases:
| party | market id | alias |
| vamm1 | ETH/MAR22 | vamm1-acc |
And the following transfers should happen:
| from | from account | to | to account | market id | amount | asset | is amm | type |
| from | from account | to | to account | market id | amount | asset | is amm | type |
| vamm1 | ACCOUNT_TYPE_GENERAL | vamm1-acc | ACCOUNT_TYPE_GENERAL | | 100000 | USD | true | TRANSFER_TYPE_AMM_LOW |

4 changes: 2 additions & 2 deletions core/integration/features/amm/0090-VAMM-003.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ Feature: Test vAMM submission works as expected
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 150 | 0.25 | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | upper bound | upper leverage |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 150 | 0.25 |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 150 | 0.25 |

And set the following AMM sub account aliases:
| party | market id | alias |
| vamm1 | ETH/MAR22 | vamm1-acc |
And the following transfers should happen:
| from | from account | to | to account | market id | amount | asset | is amm | type |
| from | from account | to | to account | market id | amount | asset | is amm | type |
| vamm1 | ACCOUNT_TYPE_GENERAL | vamm1-acc | ACCOUNT_TYPE_GENERAL | | 100000 | USD | true | TRANSFER_TYPE_AMM_LOW |

20 changes: 10 additions & 10 deletions core/integration/features/amm/0090-VAMM-004.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,35 +98,35 @@ Feature: Test vAMM submission works as expected (invalid submission)

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | error | proposed fee |
| vamm1 | ETH/MAR22 | 200000 | 0.1 | 90 | 85 | 0.25 | not enough collateral in general account | 0.01 |
| vamm1 | ETH/MAR22 | 200000 | 0.1 | 90 | 85 | 0.25 | not enough collateral in general account | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage | reason |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 90 | 85 | 0.25 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 90 | 85 | 0.25 | STATUS_REASON_CANNOT_FILL_COMMITMENT |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | upper bound | upper leverage | error | proposed fee |
| vamm1 | ETH/MAR22 | 200000 | 0.1 | 110 | 150 | 0.25 | not enough collateral in general account | 0.01 |
| vamm1 | ETH/MAR22 | 200000 | 0.1 | 110 | 150 | 0.25 | not enough collateral in general account | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | upper bound | upper leverage | reason |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 110 | 150 | 0.25 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 110 | 150 | 0.25 | STATUS_REASON_CANNOT_FILL_COMMITMENT |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | error | proposed fee |
| vamm1 | ETH/MAR22 | 200000 | 0.01 | 110 | 99 | 0.1 | not enough collateral in general account | 0.01 |
| vamm1 | ETH/MAR22 | 200000 | 0.01 | 110 | 99 | 0.1 | not enough collateral in general account | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage | reason |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 110 | 99 | 0.1 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 110 | 99 | 0.1 | STATUS_REASON_CANNOT_FILL_COMMITMENT |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | upper bound | upper leverage | error | proposed fee |
| vamm1 | ETH/MAR22 | 200000 | 0.01 | 90 | 101 | 0.02 | not enough collateral in general account | 0.01 |
| vamm1 | ETH/MAR22 | 200000 | 0.01 | 90 | 101 | 0.02 | not enough collateral in general account | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | upper bound | upper leverage | reason |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 90 | 101 | 0.02 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 90 | 101 | 0.02 | STATUS_REASON_CANNOT_FILL_COMMITMENT |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | upper bound | lower leverage | upper leverage | error | proposed fee |
| vamm1 | ETH/MAR22 | 200000 | 0.001 | 101 | 95 | 105 | 0.01 | 0.01 | not enough collateral in general account | 0.01 |
| vamm1 | ETH/MAR22 | 200000 | 0.001 | 101 | 95 | 105 | 0.01 | 0.01 | not enough collateral in general account | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage | upper bound | upper leverage | reason |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 101 | 95 | 0.01 | 105 | 0.01 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
| vamm1 | ETH/MAR22 | 200000 | STATUS_REJECTED | 101 | 95 | 0.01 | 105 | 0.01 | STATUS_REASON_CANNOT_FILL_COMMITMENT |
22 changes: 11 additions & 11 deletions core/integration/features/amm/0090-VAMM-005.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,25 +97,25 @@ Feature: Test vAMM submission works as expected (invalid submission)

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | upper bound | lower leverage | upper leverage | proposed fee |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 150 | 0.25 | 0.25 | 0.01 |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 100 | 85 | 150 | 0.25 | 0.25 | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage | upper bound | upper leverage |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 0.25 | 150 | 0.25 |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 100 | 85 | 0.25 | 150 | 0.25 |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | error | proposed fee |
| vamm2 | ETH/MAR22 | 100000 | 0.1 | 90 | 85 | 0.25 | rebase target outside bounds | 0.01 |
| vamm2 | ETH/MAR22 | 100000 | 0.1 | 90 | 85 | 0.25 | rebase target outside bounds | 0.01 |
# can't rebase because the target is 100 and thats outside of its bounds given there is no upper
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage |
| vamm2 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 90 | 85 | 0.25 |
| party | market id | amount | status | base | lower bound | lower leverage |
| vamm2 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 90 | 85 | 0.25 |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | upper bound | upper leverage | error | proposed fee |
| vamm3 | ETH/MAR22 | 100000 | 0.1 | 110 | 150 | 0.25 | rebase target outside bounds | 0.01 |
| vamm3 | ETH/MAR22 | 100000 | 0.1 | 110 | 150 | 0.25 | rebase target outside bounds | 0.01 |
Then the AMM pool status should be:
| party | market id | amount | status | base | upper bound | upper leverage |
| vamm3 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 110 | 150 | 0.25 |
| party | market id | amount | status | base | upper bound | upper leverage |
| vamm3 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 110 | 150 | 0.25 |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | proposed fee |
Expand All @@ -133,10 +133,10 @@ Feature: Test vAMM submission works as expected (invalid submission)

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | upper bound | lower leverage | upper leverage | proposed fee | error |
| vamm6 | ETH/MAR22 | 100000 | 0.001 | 101 | 95 | 105 | 0.01 | 0.01 | 0.01 | blah |
| vamm6 | ETH/MAR22 | 100000 | 0.001 | 101 | 95 | 105 | 0.01 | 0.01 | 0.01 | blah |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | lower leverage | upper bound | upper leverage |
| vamm6 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 101 | 95 | 0.01 | 105 | 0.01 |
| party | market id | amount | status | base | lower bound | lower leverage | upper bound | upper leverage |
| vamm6 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 101 | 95 | 0.01 | 105 | 0.01 |

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | lower leverage | proposed fee |
Expand Down
6 changes: 3 additions & 3 deletions core/integration/features/amm/0090-VAMM-015.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,16 @@ Feature: Ensure the vAMM contributes to fee factor setting

When the parties submit the following AMM:
| party | market id | amount | slippage | base | lower bound | upper bound | lower leverage | upper leverage | proposed fee |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 80 | 65 | 130 | 0.25 | 0.25 | 0.03 |
| vamm1 | ETH/MAR22 | 100000 | 0.1 | 80 | 65 | 130 | 0.25 | 0.25 | 0.03 |
Then the AMM pool status should be:
| party | market id | amount | status | base | lower bound | upper bound | lower leverage | upper leverage |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 80 | 65 | 130 | 0.25 | 0.25 |
| vamm1 | ETH/MAR22 | 100000 | STATUS_ACTIVE | 80 | 65 | 130 | 0.25 | 0.25 |

And set the following AMM sub account aliases:
| party | market id | alias |
| vamm1 | ETH/MAR22 | vamm1-id |
And the following transfers should happen:
| from | from account | to | to account | market id | amount | asset | is amm | type |
| from | from account | to | to account | market id | amount | asset | is amm | type |
| vamm1 | ACCOUNT_TYPE_GENERAL | vamm1-id | ACCOUNT_TYPE_GENERAL | | 100000 | USD | true | TRANSFER_TYPE_AMM_LOW |

When the network moves ahead "4" blocks
Expand Down
Loading
Loading