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: mempool by nonce removal during iteration #2785

Merged
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
7 changes: 4 additions & 3 deletions packages/chain/mempool/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,9 @@ func (mpi *mempoolImpl) refsToPropose() []*isc.RequestRef {
reqNonce := e.req.Nonce()
if reqNonce < accountNonce {
// nonce too old, delete
mpi.log.Debugf("refsToPropose, account: %s, removing old nonce from pool: %d", account, e.req.Nonce())
mpi.log.Debugf("refsToPropose, account: %s, removing request (%s) with old nonce (%d) from the pool", account, e.req.ID(), e.req.Nonce())
mpi.offLedgerPool.Remove(e.req)
continue
}
if e.old {
// this request was marked as "old", do not propose it
Expand All @@ -565,12 +566,12 @@ func (mpi *mempoolImpl) refsToPropose() []*isc.RequestRef {
}
if reqNonce == accountNonce {
// expected nonce, add it to the list to propose
mpi.log.Debugf("refsToPropose, account: %s, proposing reqID %s with nonce %d: d", account, e.req.ID().String(), e.req.Nonce())
mpi.log.Debugf("refsToPropose, account: %s, proposing reqID %s with nonce: %d", account, e.req.ID().String(), e.req.Nonce())
reqRefs = append(reqRefs, isc.RequestRefFromRequest(e.req))
accountNonce++ // increment the account nonce to match the next valid request
}
if reqNonce > accountNonce {
mpi.log.Debugf("refsToPropose, account: %s, req %s has a nonce %d which is too high, won't be proposed", account, e.req.ID().String(), e.req.Nonce())
mpi.log.Debugf("refsToPropose, account: %s, req %s has a nonce %d which is too high (expected %d), won't be proposed", account, e.req.ID().String(), e.req.Nonce(), accountNonce)
return // no more valid nonces for this account, continue to the next account
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/chain/mempool/typed_pool_by_nonce.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (p *TypedPoolByNonce[V]) Remove(request V) {
}
// find the request in the accounts map
indexToDel := slices.IndexFunc(reqsByAccount, func(e *OrderedPoolEntry[V]) bool {
return true
return refKey == isc.RequestRefFromRequest(e.req).AsKey()
})
if indexToDel == -1 {
p.log.Error("inconsistency trying to DEL %v as key=%v, request not found in list for account %s", request.ID(), refKey, account)
Expand All @@ -148,8 +148,8 @@ func (p *TypedPoolByNonce[V]) Remove(request V) {
}

func (p *TypedPoolByNonce[V]) Iterate(f func(account string, requests []*OrderedPoolEntry[V])) {
p.reqsByAcountOrdered.ForEach(func(acc string, reqs []*OrderedPoolEntry[V]) bool {
f(acc, reqs)
p.reqsByAcountOrdered.ForEach(func(acc string, entries []*OrderedPoolEntry[V]) bool {
f(acc, slices.Clone(entries))
return true
})
}
Expand Down
51 changes: 51 additions & 0 deletions packages/chain/mempool/typed_pool_by_nonce_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package mempool

import (
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/iotaledger/wasp/packages/isc"
"github.com/iotaledger/wasp/packages/testutil"
"github.com/iotaledger/wasp/packages/testutil/testkey"
"github.com/iotaledger/wasp/packages/testutil/testlogger"
)

func TestSomething(t *testing.T) {
waitReq := NewWaitReq(waitRequestCleanupEvery)
pool := NewTypedPoolByNonce[isc.OffLedgerRequest](waitReq, func(int) {}, func(time.Duration) {}, testlogger.NewSilentLogger("", true))

// generate a bunch of requests for the same account
kp, addr := testkey.GenKeyAddr()
agentID := isc.NewAgentID(addr)

req0 := testutil.DummyOffledgerRequestForAccount(isc.RandomChainID(), 0, kp)
req1 := testutil.DummyOffledgerRequestForAccount(isc.RandomChainID(), 1, kp)
req2 := testutil.DummyOffledgerRequestForAccount(isc.RandomChainID(), 2, kp)
req2new := testutil.DummyOffledgerRequestForAccount(isc.RandomChainID(), 2, kp)
pool.Add(req0)
pool.Add(req1)
pool.Add(req1) // try to add the same request many times
pool.Add(req2)
pool.Add(req1)
require.EqualValues(t, 3, pool.refLUT.Size())
require.EqualValues(t, 1, pool.reqsByAcountOrdered.Size())
reqsInPoolForAccount, _ := pool.reqsByAcountOrdered.Get(agentID.String())
require.Len(t, reqsInPoolForAccount, 3)
pool.Add(req2new)
pool.Add(req2new)
require.EqualValues(t, 4, pool.refLUT.Size())
require.EqualValues(t, 1, pool.reqsByAcountOrdered.Size())
reqsInPoolForAccount, _ = pool.reqsByAcountOrdered.Get(agentID.String())
require.Len(t, reqsInPoolForAccount, 4)

// try to remove everything during iteration
pool.Iterate(func(account string, entries []*OrderedPoolEntry[isc.OffLedgerRequest]) {
for _, e := range entries {
pool.Remove(e.req)
}
})
require.EqualValues(t, 0, pool.refLUT.Size())
require.EqualValues(t, 0, pool.reqsByAcountOrdered.Size())
}
9 changes: 9 additions & 0 deletions packages/testutil/dummyrequest.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package testutil

import (
"github.com/iotaledger/wasp/packages/cryptolib"
"github.com/iotaledger/wasp/packages/isc"
"github.com/iotaledger/wasp/packages/kv/dict"
"github.com/iotaledger/wasp/packages/testutil/testkey"
Expand All @@ -15,3 +16,11 @@ func DummyOffledgerRequest(chainID isc.ChainID) isc.OffLedgerRequest {
keys, _ := testkey.GenKeyAddr()
return req.Sign(keys)
}

func DummyOffledgerRequestForAccount(chainID isc.ChainID, nonce uint64, kp *cryptolib.KeyPair) isc.OffLedgerRequest {
contract := isc.Hn("somecontract")
entrypoint := isc.Hn("someentrypoint")
args := dict.Dict{}
req := isc.NewOffLedgerRequest(chainID, contract, entrypoint, args, nonce, gas.LimitsDefault.MaxGasPerRequest)
return req.Sign(kp)
}
Loading