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

Trim the transaction table #1061

Merged
merged 31 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e3ca104
track lf acc nonce via bs instead of tt.
MilkywayPirate Oct 31, 2023
bab26a2
Merge branch 'lmdb-accountmap' into trim-tt
Nov 6, 2023
4ea9f71
Purge accounts from anft table when they do not have any pending tran…
Nov 6, 2023
de4d4de
Queries now take nextNonce potentially by bs if no entry in tt.
Nov 6, 2023
a648ee5
Merge branch 'lmdb-accountmap' into trim-tt
Nov 10, 2023
42b0e65
revert.
Nov 10, 2023
15eca04
Change some constraints in skovdata impl (consensus version 1)
MilkywayPirate Nov 11, 2023
210be34
fix wrong nonce check
MilkywayPirate Nov 11, 2023
b54be71
Merge branch 'main' into trim-tt
MilkywayPirate Nov 13, 2023
2a93b10
do not retain accounts in transaction table when they do not have any
MilkywayPirate Nov 14, 2023
7f987ff
reintroduce some tests.
MilkywayPirate Nov 14, 2023
b6eaa3a
Merge branch 'main' into trim-tt
MilkywayPirate Nov 14, 2023
3e36c68
fix some unnecessary changes.
MilkywayPirate Nov 14, 2023
d58d0e2
Cleanup some documentation.
MilkywayPirate Nov 14, 2023
77b3d0e
fmt.
MilkywayPirate Nov 14, 2023
3db1741
Correctly set anftNextNonce after a purge.
Nov 15, 2023
82f1271
Add integration test that checks that anftNextNonce is correct.
Nov 15, 2023
e66076e
Fix doc and changelog.
Nov 15, 2023
310369b
Tweaks to purging.
MilkywayPirate Nov 15, 2023
df0e8d3
changelog fix.
MilkywayPirate Nov 15, 2023
8070111
Cleanup transaction table and finalization of transactions logic.
MilkywayPirate Nov 16, 2023
cdc4c50
cleanup tests.
MilkywayPirate Nov 16, 2023
4c1da50
remove anftNextNonce, make sure to cache account table and some cleanup.
MilkywayPirate Nov 16, 2023
c007653
Fix tests
MilkywayPirate Nov 16, 2023
1992ed4
A few refactorings and more accurate documentation.
MilkywayPirate Nov 17, 2023
27192f8
Re-check nonce before adding pre-verified transaction into transactio…
MilkywayPirate Nov 17, 2023
7c3f13c
Address review comments.
MilkywayPirate Nov 17, 2023
e5fb993
Be explicit about where to get next nonce from when performing extra
MilkywayPirate Nov 17, 2023
038b8bd
Cleanup
MilkywayPirate Nov 17, 2023
eef74a7
Move documentation a bit.
MilkywayPirate Nov 17, 2023
c4d6d1d
Make purging more strict and fix some documentation.
MilkywayPirate Nov 17, 2023
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## Unreleased changes
- If an account does not have any non-finalized transactions, then the transaction table is no longer used for tracking next available account nonce.

## 6.2.1

Expand Down
4 changes: 2 additions & 2 deletions concordium-consensus/src/Concordium/GlobalState/BlockState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,8 +1437,8 @@ class (BlockStateOperations m, FixedSizeSerialization (BlockStateRef m)) => Bloc
-- | Cache the block state.
cacheBlockState :: BlockState m -> m ()

-- | Cache the block state and get the initial (empty) transaction table with the next account nonces
-- and update sequence numbers populated.
-- | Cache the block state and get the initial (empty) transaction table with
-- the next "update sequence numbers".
cacheBlockStateAndGetTransactionTable :: BlockState m -> m TransactionTable

-- | Populate the LMDB account map if it has not already been initialized.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3789,22 +3789,7 @@ cacheStateAndGetTransactionTable ::
m TransactionTable.TransactionTable
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
cacheStateAndGetTransactionTable hpbs = do
BlockStatePointers{..} <- loadPBS (hpbsPointers hpbs)
-- When caching the accounts, we populate the transaction table with the next account nonces.
-- This is done by using 'liftCache' on the account table with a custom cache function that
-- records the nonces.
let perAcct acct = do
-- Note: we do not need to cache the account because a loaded account is already fully
-- cached. (Indeed, 'cache' is defined to be 'pure'.)
nonce <- accountNonce acct
unless (nonce == minNonce) $ do
addr <- accountCanonicalAddress acct
MTL.modify
( TransactionTable.ttNonFinalizedTransactions . at' (accountAddressEmbed addr)
?~ TransactionTable.emptyANFTWithNonce nonce
)
return acct
(accts, tt0) <- MTL.runStateT (liftCache perAcct bspAccounts) TransactionTable.emptyTransactionTable
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
-- first cache the modules
-- cache the modules
mods <- cache bspModules
-- then cache the instances, but don't cache the modules again. Instead
-- share the references in memory we have already constructed by caching
Expand All @@ -3825,14 +3810,14 @@ cacheStateAndGetTransactionTable hpbs = do
& TransactionTable.ttNonFinalizedChainUpdates . at' uty
?~ TransactionTable.emptyNFCUWithSequenceNumber sn
else return tt
tt <- foldM updInTT tt0 [minBound ..]
tt <- foldM updInTT TransactionTable.emptyTransactionTable [minBound ..]
rels <- cache bspReleaseSchedule
red <- cache bspRewardDetails
_ <-
storePBS
(hpbsPointers hpbs)
BlockStatePointers
{ bspAccounts = accts,
{ bspAccounts = bspAccounts,
bspInstances = insts,
bspModules = mods,
bspBank = bspBank,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,17 +634,37 @@ constructBlock StoredBlockWithStateHash{sbshStoredBlock = StoredBlock{..}, ..} =

instance
( MonadState state m,
HasSkovPersistentData pv state
HasSkovPersistentData pv state,
BlockStateQuery m,
BlockState m ~ PBS.HashedPersistentBlockState pv,
MonadProtocolVersion m,
MPV m ~ pv,
MonadLogger (PersistentTreeStateMonad state m),
MonadIO (PersistentTreeStateMonad state m),
BlockStateStorage (PersistentTreeStateMonad state m)
) =>
AccountNonceQuery (PersistentTreeStateMonad state m)
where
getNextAccountNonce addr = nextAccountNonce addr <$> use (skovPersistentData . transactionTable)
getNextAccountNonce addr =
fetchFromTransactionTable >>= \case
Just ttResult -> return ttResult
Nothing -> fetchFromLastFinalizedBlock
where
fetchFromTransactionTable = nextAccountNonce addr <$> use (skovPersistentData . transactionTable)
fetchFromLastFinalizedBlock = do
lfb <- use (skovPersistentData . lastFinalized)
st <- blockState lfb
macct <- getAccount st (aaeAddress addr)
nextNonce <- fromMaybe minNonce <$> mapM (getAccountNonce . snd) macct
return (nextNonce, True)
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
{-# INLINE getNextAccountNonce #-}

instance
( MonadLogger (PersistentTreeStateMonad state m),
MonadIO (PersistentTreeStateMonad state m),
BlockStateStorage (PersistentTreeStateMonad state m),
MonadState state m,
BlockStateQuery m,
HasSkovPersistentData pv state,
MonadProtocolVersion m,
MPV m ~ pv,
Expand Down Expand Up @@ -901,9 +921,11 @@ instance
finTrans WithMetadata{wmdData = NormalTransaction tr, ..} = do
let nonce = transactionNonce tr
sender = accountAddressEmbed (transactionSender tr)
anft <- use (skovPersistentData . transactionTable . ttNonFinalizedTransactions . at' sender . non emptyANFT)
if anft ^. anftNextNonce == nonce
then do
tt <- use (skovPersistentData . transactionTable)
let mAnft = tt ^? ttNonFinalizedTransactions . ix sender
case mAnft of
Nothing -> logErrorAndThrowTS $ "When finalizing transactions there was no recorded next nonce for sender " <> show sender
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
Just anft -> do
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
let nfn = anft ^. anftMap . at' nonce . non Map.empty
wmdtr = WithMetadata{wmdData = tr, ..}
if Map.member wmdtr nfn
Expand All @@ -919,16 +941,13 @@ instance
. transactionTable
. ttNonFinalizedTransactions
. at' sender
?= ( anft
?=! ( anft
& (anftMap . at' nonce .~ Nothing)
& (anftNextNonce .~ nonce + 1)
)
)
return ss
else do
logErrorAndThrowTS $ "Tried to finalize transaction which is not known to be in the set of non-finalized transactions for the sender " ++ show sender
else do
logErrorAndThrowTS $
"The recorded next nonce for the account " ++ show sender ++ " (" ++ show (anft ^. anftNextNonce) ++ ") doesn't match the one that is going to be finalized (" ++ show nonce ++ ")"
finTrans WithMetadata{wmdData = CredentialDeployment{}, ..} =
deleteAndFinalizeStatus wmdHash
finTrans WithMetadata{wmdData = ChainUpdate cu, ..} = do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,32 @@ purgeTables lastFinCommitPoint oldestArrivalTime currentTime TransactionTable{..
| otherwise = (mmnonce <> Just (Max n), Just ts')
put (mmnonce', tht')
return mres
-- Purge the non-finalized transactions for a specific account.
purgeAccount :: AccountAddressEq -> AccountNonFinalizedTransactions -> State (PendingTransactionTable, TransactionHashTable) AccountNonFinalizedTransactions
purgeAccount addr AccountNonFinalizedTransactions{..} = do
(ptt0, trs0) <- get
-- Purge the non-finalized transactions for a specific account by
-- collect non-finalized transactions for all accounts.
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
purgeAccount ::
-- accummulator
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
( HM.HashMap AccountAddressEq AccountNonFinalizedTransactions,
(PendingTransactionTable, TransactionHashTable)
) ->
-- current entry key
AccountAddressEq ->
-- current entry value
AccountNonFinalizedTransactions ->
-- result
( HM.HashMap AccountAddressEq AccountNonFinalizedTransactions,
(PendingTransactionTable, TransactionHashTable)
)
purgeAccount (anfts, (ptt0, trs0)) addr AccountNonFinalizedTransactions{..} =
-- Purge the transactions from the transaction table.
let (newANFTMap, (mmax, !trs1)) = runState (Map.traverseMaybeWithKey purgeTxs _anftMap) (Nothing, trs0)
-- Update the pending transaction table.
let updptt (Just (Max newHigh)) (Just (low, _))
-- Update the pending transaction table.
updptt (Just (Max newHigh)) (Just (low, _))
| newHigh < low = Nothing
| otherwise = Just (low, newHigh)
updptt _ _ = Nothing
!ptt1 = ptt0 & pttWithSender . at' addr %~ updptt mmax
put (ptt1, trs1)
return AccountNonFinalizedTransactions{_anftMap = newANFTMap, ..}
anfts' = if null newANFTMap then anfts else HM.insert addr AccountNonFinalizedTransactions{_anftMap = newANFTMap, ..} anfts
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
in (anfts', (ptt1, trs1))
-- Purge the deploy credential transactions that are pending.
purgeDeployCredentials = do
dc0 <- use (_1 . pttDeployCredential)
Expand Down Expand Up @@ -175,8 +187,11 @@ purgeTables lastFinCommitPoint oldestArrivalTime currentTime TransactionTable{..
!ptt1 = ptt0 & pttUpdates . at' uty %~ updptt mmax
in (nfcu{_nfcuMap = newNFCUMap}, (ptt1, uis1))
purge = do
-- Purge each account
nnft <- HM.traverseWithKey purgeAccount _ttNonFinalizedTransactions
-- Purge each account, and possibly remove the @AccountNonFinalizedTransactions@
-- if an account does not have any pending transactions left.
s <- get
let (nnft, s') = HM.foldlWithKey' purgeAccount (HM.empty, s) _ttNonFinalizedTransactions
put s'
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
-- Purge credential deployments
purgeDeployCredentials
-- Purge chain updates
Expand Down
38 changes: 24 additions & 14 deletions concordium-consensus/src/Concordium/GlobalState/TransactionTable.hs
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,13 @@ emptyNFCUWithSequenceNumber = NonFinalizedChainUpdates Map.empty
-- may also have a non-zero highest commit point if it is received in a block, but that block
-- is not yet considered arrived (e.g. it is pending its parent).
--
-- Generally, '_ttNonFinalizedTransactions' should have an entry for every account,
-- with the exception of where the entry would be 'emptyANFT'. Similarly with
-- '_ttNonFinalizedChainUpdates' and 'emptyNFCU'. In particular, there should be
-- an entry if the next nonce/sequence number is not the minimum value.
-- The '_ttNonFinalizedTransactions' should have an entry for every account which have non-finalized transactions.
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
-- The '_ttNonFinalizedChainUpdates' should have an entry for all kinds of 'UpdateType's with the exception of where it would be 'emptyNFCU'.
-- In particular, there should be an entry if the next nonce/sequence number is not the minimum value.
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
data TransactionTable = TransactionTable
{ -- | Map from transaction hashes to transactions, together with their current status.
_ttHashMap :: !(HM.HashMap TransactionHash (BlockItem, LiveTransactionStatus)),
-- | For each account, the non-finalized transactions for that account,
-- | For accounts that have non-finalized transactions, the non-finalized transactions for that account,
-- grouped by nonce. See $equivalence for reasons why AccountAddressEq is used.
_ttNonFinalizedTransactions :: !(HM.HashMap AccountAddressEq AccountNonFinalizedTransactions),
-- | For each update types, the non-finalized update instructions, grouped by
Expand Down Expand Up @@ -260,16 +259,26 @@ emptyTransactionTableWithSequenceNumbers accs upds =
-- | Add a transaction to a transaction table if its nonce/sequence number is at least the next
-- non-finalized nonce/sequence number. A return value of 'True' indicates that the transaction
-- was added. The caller should check that the transaction is not already present.
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
addTransaction :: BlockItem -> CommitPoint -> TVer.VerificationResult -> TransactionTable -> (Bool, TransactionTable)
addTransaction ::
-- | Transaction to add.
BlockItem ->
-- | Commit point of the transaction.
CommitPoint ->
-- | The associated verification result.
TVer.VerificationResult ->
-- | The transaction table to update.
TransactionTable ->
-- | First component is @True@ if table was updated.
-- Second component is the updated table.
(Bool, TransactionTable)
addTransaction blockItem@WithMetadata{..} cp !verRes tt0 =
case wmdData of
NormalTransaction tr
| tt0 ^. senderANFT . anftNextNonce <= nonce ->
(True, tt1 & senderANFT . anftMap . at' nonce . non Map.empty . at' wmdtr ?~ verRes)
NormalTransaction tr -> (True, tt1 & senderANFT . anftMap . at' nonce . non Map.empty . at' wmdtr ?~ verRes)
where
sender = accountAddressEmbed (transactionSender tr)
senderANFT :: Lens' TransactionTable AccountNonFinalizedTransactions
senderANFT = ttNonFinalizedTransactions . at' sender . non emptyANFT
senderANFT = ttNonFinalizedTransactions . at' sender . non anft
anft = emptyANFTWithNonce nonce
nonce = transactionNonce tr
wmdtr = WithMetadata{wmdData = tr, ..}
CredentialDeployment{} -> (True, tt1)
Expand Down Expand Up @@ -450,19 +459,20 @@ reversePTT trs ptt0 = foldr reverse1 ptt0 trs
-- provided account address in the first component and the
-- 'Bool' in the second component is 'True' only if all transactions from the
-- provided account are finalized.
-- Returns @Nothing@ if no non-finalized transactions were recorded for the provided account.
nextAccountNonce ::
-- | The account to look up the next account nonce for.
AccountAddressEq ->
-- | The transaction table to look up in.
TransactionTable ->
-- | ("the next available account nonce", "whether all transactions from the account are finalized").
(Nonce, Bool)
Maybe (Nonce, Bool)
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
nextAccountNonce addr tt = case tt ^. ttNonFinalizedTransactions . at' addr of
Nothing -> (minNonce, True)
Nothing -> Nothing
Just anfts ->
case Map.lookupMax (anfts ^. anftMap) of
Nothing -> (anfts ^. anftNextNonce, True)
Just (nonce, _) -> (nonce + 1, False)
Nothing -> Just (anfts ^. anftNextNonce, True)
MilkywayPirate marked this conversation as resolved.
Show resolved Hide resolved
Just (nonce, _) -> Just (nonce + 1, False)

-- * Transaction grouping

Expand Down
Loading
Loading