Skip to content

Commit

Permalink
fix: do not remove from slice with iterating over it in amm engine
Browse files Browse the repository at this point in the history
  • Loading branch information
wwestgarth committed May 10, 2024
1 parent 4c84246 commit cc6417f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 11 deletions.
16 changes: 12 additions & 4 deletions core/execution/amm/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ func (e *Engine) OnMinCommitmentQuantumUpdate(ctx context.Context, c *num.Uint)

// OnMTM is called whenever core does an MTM and is a signal that any pool's that are closing and have 0 position can be fully removed.
func (e *Engine) OnMTM(ctx context.Context) {
rm := []string{}
for _, p := range e.poolsCpy {
if !p.closing() {
continue
Expand All @@ -271,7 +272,10 @@ func (e *Engine) OnMTM(ctx context.Context) {
e.log.Error("unable to release subaccount balance", logging.Error(err))
}
p.status = types.AMMPoolStatusCancelled
e.remove(ctx, p.party)
rm = append(rm, p.party)
}
for _, party := range rm {
e.remove(ctx, party)
}
}

Expand All @@ -281,13 +285,16 @@ func (e *Engine) OnTick(ctx context.Context, _ time.Time) {
e.idgen = idgeneration.New(blockHash + crypto.HashStrToHex("amm-engine"+e.market.GetID()))

// any pools that for some reason have zero balance in their accounts will get stopped
rm := []string{}
for _, p := range e.poolsCpy {
if p.getBalance().IsZero() {
p.status = types.AMMPoolStatusStopped
e.remove(ctx, p.party)
continue
rm = append(rm, p.party)
}
}
for _, party := range rm {
e.remove(ctx, party)
}
}

// RemoveDistressed checks if any of the closed out parties are AMM's and if so the AMM is stopped and removed.
Expand Down Expand Up @@ -780,7 +787,8 @@ func (e *Engine) MarketClosing(ctx context.Context) error {
return err
}
p.status = types.AMMPoolStatusStopped
e.remove(ctx, p.party)
e.sendUpdate(ctx, p)
e.marketActivityTracker.RemoveAMMSubAccount(e.market.GetSettlementAsset(), e.market.GetID(), p.SubAccount)
}
return nil
}
Expand Down
16 changes: 10 additions & 6 deletions core/execution/amm/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,15 +647,19 @@ func testMarketClosure(t *testing.T) {
ctx := vgcontext.WithTraceID(context.Background(), vgcrypto.RandomHash())
tst := getTestEngine(t)

party, subAccount := getParty(t, tst)
submit := getPoolSubmission(t, party, tst.marketID)
for i := 0; i < 10; i++ {
party, subAccount := getParty(t, tst)
submit := getPoolSubmission(t, party, tst.marketID)

expectSubaccountCreation(t, tst, party, subAccount)
require.NoError(t, tst.engine.SubmitAMM(ctx, submit, vgcrypto.RandomHash(), nil))
expectSubaccountCreation(t, tst, party, subAccount)
require.NoError(t, tst.engine.SubmitAMM(ctx, submit, vgcrypto.RandomHash(), nil))
expectSubAccountClose(t, tst, party, subAccount)
}

expectSubAccountClose(t, tst, party, subAccount)
require.NoError(t, tst.engine.MarketClosing(ctx))
assert.Len(t, tst.engine.poolsCpy, 0)
for _, p := range tst.engine.poolsCpy {
assert.Equal(t, types.AMMPoolStatusStopped, p.status)
}
}

func expectSubaccountCreation(t *testing.T, tst *tstEngine, party, subAccount string) {
Expand Down
5 changes: 5 additions & 0 deletions core/execution/common/liquidity_provision_fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ func (m *MarketLiquidity) scaleScores(scores map[string]num.Decimal) map[string]
for _, s := range scores {
total = total.Add(s)
}

if total.IsZero() {
return scores
}

for k, v := range scores {
scores[k] = v.Div(total)
}
Expand Down
2 changes: 1 addition & 1 deletion datanode/networkhistory/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.d
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package networkhistory_test

Expand Down

0 comments on commit cc6417f

Please sign in to comment.