From b0675e21b6c3ead38d562360812fba4a06dc4437 Mon Sep 17 00:00:00 2001 From: Piotr Macek <4007944+piotrm50@users.noreply.github.com> Date: Mon, 6 May 2024 15:33:27 +0200 Subject: [PATCH] Fix snapshot generation for destroyed accounts. --- .../engine/accounts/accountsledger/manager.go | 60 ++++++++++++------- .../accounts/accountsledger/snapshot.go | 29 ++++----- .../accounts/accountsledger/snapshot_test.go | 2 +- .../prunable/slotstore/accountdiffs.go | 30 +++++++--- 4 files changed, 71 insertions(+), 50 deletions(-) diff --git a/pkg/protocol/engine/accounts/accountsledger/manager.go b/pkg/protocol/engine/accounts/accountsledger/manager.go index 42f86f6a6..d426ecc85 100644 --- a/pkg/protocol/engine/accounts/accountsledger/manager.go +++ b/pkg/protocol/engine/accounts/accountsledger/manager.go @@ -39,8 +39,7 @@ type Manager struct { // at the latest committed slot, it is updated on the slot commitment. accountsTree ads.Map[iotago.Identifier, iotago.AccountID, *accounts.AccountData] - // TODO: add in memory shrink version of the slot diffs - // slot diffs for the Account between [LatestCommittedSlot - MCA, LatestCommittedSlot]. + // slot diffs for the Account. slotDiff func(iotago.SlotIndex) (*slotstore.AccountDiffs, error) // block is a function that returns a block from the cache or from the database. @@ -228,15 +227,16 @@ func (m *Manager) account(accountID iotago.AccountID, targetSlot iotago.SlotInde } if !exists { - loadedAccount = accounts.NewAccountData(accountID, accounts.WithCredits(accounts.NewBlockIssuanceCredits(0, targetSlot))) + loadedAccount = accounts.NewAccountData(accountID) } - _, wasDestroyed, err := m.rollbackAccountTo(loadedAccount, targetSlot) + wasDestroyed, err := m.rollbackAccountTo(loadedAccount, targetSlot) if err != nil { return nil, false, err } - // account not present in the accountsTree, and it was not marked as destroyed in slots between targetSlot and latestCommittedSlot + // the account is not present in the accountsTree, + // and it was not marked as destroyed in slots between targetSlot and latestCommittedSlot if !exists && !wasDestroyed { return nil, false, nil } @@ -261,7 +261,7 @@ func (m *Manager) PastAccounts(accountIDs iotago.AccountIDs, targetSlot iotago.S if !exists { loadedAccount = accounts.NewAccountData(accountID, accounts.WithCredits(accounts.NewBlockIssuanceCredits(0, targetSlot))) } - _, wasDestroyed, err := m.rollbackAccountTo(loadedAccount, targetSlot) + wasDestroyed, err := m.rollbackAccountTo(loadedAccount, targetSlot) if err != nil { continue } @@ -276,15 +276,17 @@ func (m *Manager) PastAccounts(accountIDs iotago.AccountIDs, targetSlot iotago.S return result, nil } - func (m *Manager) Rollback(targetSlot iotago.SlotIndex) error { - for slot := m.latestCommittedSlot; slot > targetSlot; slot-- { + return m.rollbackFromTo(m.latestCommittedSlot, targetSlot, false) +} + +func (m *Manager) rollbackFromTo(fromSlot iotago.SlotIndex, toSlot iotago.SlotIndex, deleteRevertedDiffs bool) error { + for slot := fromSlot; slot > toSlot; slot-- { slotDiff := lo.PanicOnErr(m.slotDiff(slot)) var internalErr error //nolint:revive if err := slotDiff.Stream(func(accountID iotago.AccountID, accountDiff *model.AccountDiff, destroyed bool) bool { - accountData, exists, err := m.accountsTree.Get(accountID) if err != nil { internalErr = ierrors.Wrapf(err, "unable to retrieve account %s to rollback in slot %d", accountID, slot) @@ -293,16 +295,23 @@ func (m *Manager) Rollback(targetSlot iotago.SlotIndex) error { } if !exists { - // Account was not found in the tree, so we need to re-create it + // The Account was not found in the tree, so we need to re-create it accountData = accounts.NewAccountData(accountID) } - wasCreatedAfterTargetSlot, _, err := m.rollbackSlotDiffOnAccount(accountData, slotDiff) + wasCreatedAfterTargetSlot, wasDestroyed, err := m.rollbackSlotDiffOnAccount(accountData, slotDiff) if err != nil { - internalErr = ierrors.Wrapf(err, "unable to rollback account %s to target slot %d", accountID, targetSlot) + internalErr = ierrors.Wrapf(err, "unable to rollback account %s to target slot %d", accountID, toSlot) + + return false + } + + if !exists && !wasDestroyed || exists && wasDestroyed { + internalErr = ierrors.Errorf("incorrect account state %s at slot %d (exists: %t wasDestroyed: %t)", accountID, slot, exists, wasDestroyed) return false } + if wasCreatedAfterTargetSlot && exists { if _, err := m.accountsTree.Delete(accountID); err != nil { internalErr = ierrors.Wrapf(err, "failed to delete account %s from slot %d", accountID, slot) @@ -314,7 +323,7 @@ func (m *Manager) Rollback(targetSlot iotago.SlotIndex) error { } if err := m.accountsTree.Set(accountID, accountData); err != nil { - internalErr = ierrors.Wrapf(err, "failed to save rolled back account %s to target slot %d", accountID, targetSlot) + internalErr = ierrors.Wrapf(err, "failed to save rolled back account %s to target slot %d", accountID, toSlot) return false } @@ -327,6 +336,12 @@ func (m *Manager) Rollback(targetSlot iotago.SlotIndex) error { if internalErr != nil { return ierrors.Wrapf(internalErr, "error in rolling back account for slot %s", slot) } + + if deleteRevertedDiffs { + if err := slotDiff.Clear(); err != nil { + return ierrors.Wrapf(err, "error while deleting reverted diff for slot %s", slot) + } + } } if err := m.accountsTree.Commit(); err != nil { @@ -387,29 +402,30 @@ func (m *Manager) Reset() { m.latestSupportedVersionSignals.Clear() } -func (m *Manager) rollbackAccountTo(accountData *accounts.AccountData, targetSlot iotago.SlotIndex) (wasCreatedAfterTargetSlot bool, wasDestroyed bool, err error) { +func (m *Manager) rollbackAccountTo(accountData *accounts.AccountData, targetSlot iotago.SlotIndex) (wasDestroyed bool, err error) { // to reach targetSlot, we need to rollback diffs from the current latestCommittedSlot down to targetSlot + 1 for diffSlot := m.latestCommittedSlot; diffSlot > targetSlot; diffSlot-- { diffStore, err := m.slotDiff(diffSlot) if err != nil { - return false, false, ierrors.Errorf("can't retrieve account, could not find diff store for slot %d", diffSlot) + return false, ierrors.Errorf("can't retrieve account, could not find diff store for slot %d", diffSlot) } - createdAfterTargetSlot, destroyed, err := m.rollbackSlotDiffOnAccount(accountData, diffStore) + createdInTheDiff, destroyed, err := m.rollbackSlotDiffOnAccount(accountData, diffStore) if err != nil { - return false, false, ierrors.Wrapf(err, "can't retrieve account, could not rollback diff for account %s in slot %d", accountData.ID, diffStore.Slot()) - } else if createdAfterTargetSlot { - return createdAfterTargetSlot, false, nil + return false, ierrors.Wrapf(err, "can't retrieve account, could not rollback diff for account %s in slot %d", accountData.ID, diffStore.Slot()) + } else if createdInTheDiff { + // If the account was created in the diffSlot, then don't need to iterate previous slots as the account diff is definitely not there. + return false, nil } // collected to see if an account was destroyed between slotIndex and b.latestCommittedSlot index. wasDestroyed = wasDestroyed || destroyed } - return false, wasDestroyed, nil + return wasDestroyed, nil } -func (m *Manager) rollbackSlotDiffOnAccount(accountData *accounts.AccountData, diffStore *slotstore.AccountDiffs) (wasCreatedAfterTargetSlot bool, wasDestroyed bool, err error) { +func (m *Manager) rollbackSlotDiffOnAccount(accountData *accounts.AccountData, diffStore *slotstore.AccountDiffs) (wasCreatedInTheDiff bool, wasDestroyed bool, err error) { found, err := diffStore.Has(accountData.ID) if err != nil { return false, false, ierrors.Wrapf(err, "can't retrieve account, could not check if diff store for slot %d has account %s", diffStore.Slot(), accountData.ID) @@ -428,7 +444,7 @@ func (m *Manager) rollbackSlotDiffOnAccount(accountData *accounts.AccountData, d m.LogDebug("Rolling back account", "accountID", accountData.ID, "diffSlot", diffStore.Slot(), "accountData", accountData, "diffChange", diffChange, "destroyed", destroyed) // update the account data with the diff - if diffChange.BICChange != 0 { + if diffChange.BICChange != 0 || destroyed { accountData.Credits.Update(-diffChange.BICChange, diffChange.PreviousUpdatedSlot) } diff --git a/pkg/protocol/engine/accounts/accountsledger/snapshot.go b/pkg/protocol/engine/accounts/accountsledger/snapshot.go index 35661c6b0..cb27ee907 100644 --- a/pkg/protocol/engine/accounts/accountsledger/snapshot.go +++ b/pkg/protocol/engine/accounts/accountsledger/snapshot.go @@ -20,12 +20,12 @@ func (m *Manager) Import(reader io.ReadSeeker) error { return ierrors.Wrap(err, "unable to read target slot") } - latestCommittedSlot, err := stream.Read[iotago.SlotIndex](reader) + accountsTreeSlot, err := stream.Read[iotago.SlotIndex](reader) if err != nil { - return ierrors.Wrap(err, "unable to read latest committed slot") + return ierrors.Wrap(err, "unable to read accounts tree slot") } - // populate the account tree, account tree should be empty at this point + // populate the account tree, the account tree should be empty at this point if err := stream.ReadCollection(reader, serializer.SeriLengthPrefixTypeAsUint64, func(i int) error { accountData, err := stream.ReadObjectFromReader(reader, accounts.AccountDataFromReader) if err != nil { @@ -47,11 +47,9 @@ func (m *Manager) Import(reader io.ReadSeeker) error { return ierrors.Wrap(err, "unable to import slot diffs") } - m.latestCommittedSlot = latestCommittedSlot - if err := m.Rollback(targetSlot); err != nil { + if err := m.rollbackFromTo(accountsTreeSlot, targetSlot, true); err != nil { return ierrors.Wrapf(err, "unable to rollback to slot %d", targetSlot) } - m.latestCommittedSlot = targetSlot return nil } @@ -140,13 +138,9 @@ func (m *Manager) readSlotDiffs(reader io.ReadSeeker) error { return ierrors.Wrapf(err, "unable to read destroyed flag for accountID %s", accountID) } - var accountDiff *model.AccountDiff - if !destroyed { - if accountDiff, err = stream.ReadObjectFromReader(reader, model.AccountDiffFromReader); err != nil { - return ierrors.Wrapf(err, "unable to read account diff for accountID %s", accountID) - } - } else { - accountDiff = model.NewAccountDiff() + accountDiff, err := stream.ReadObjectFromReader(reader, model.AccountDiffFromReader) + if err != nil { + return ierrors.Wrapf(err, "unable to read account diff for accountID %s", accountID) } m.LogDebug("Imported account diff", "slot", slot, "accountID", accountID, "destroyed", destroyed, "accountDiff", accountDiff) @@ -203,12 +197,9 @@ func (m *Manager) writeSlotDiffs(writer io.WriteSeeker, targetSlot iotago.SlotIn innerErr = ierrors.Wrapf(err, "unable to write destroyed flag for account %s", accountID) return false } - - if !destroyed { - if err = stream.WriteObject(writer, accountDiff, (*model.AccountDiff).Bytes); err != nil { - innerErr = ierrors.Wrapf(err, "unable to write account diff for account %s", accountID) - return false - } + if err = stream.WriteObject(writer, accountDiff, (*model.AccountDiff).Bytes); err != nil { + innerErr = ierrors.Wrapf(err, "unable to write account diff for account %s", accountID) + return false } m.LogDebug("Exported account diff", "slot", slot, "accountID", accountID, "destroyed", destroyed, "accountDiff", accountDiff) diff --git a/pkg/protocol/engine/accounts/accountsledger/snapshot_test.go b/pkg/protocol/engine/accounts/accountsledger/snapshot_test.go index 944c26c1c..2da28f412 100644 --- a/pkg/protocol/engine/accounts/accountsledger/snapshot_test.go +++ b/pkg/protocol/engine/accounts/accountsledger/snapshot_test.go @@ -16,7 +16,7 @@ func TestManager_Import_Export(t *testing.T) { latestSupportedVersionHash1 := tpkg.Rand32ByteArray() latestSupportedVersionHash2 := tpkg.Rand32ByteArray() - accountTreeRoots := []iotago.Identifier{} + var accountTreeRoots []iotago.Identifier accountTreeRoots = append(accountTreeRoots, ts.Instance.AccountsTreeRoot()) diff --git a/pkg/storage/prunable/slotstore/accountdiffs.go b/pkg/storage/prunable/slotstore/accountdiffs.go index f87a55381..5acdcdb65 100644 --- a/pkg/storage/prunable/slotstore/accountdiffs.go +++ b/pkg/storage/prunable/slotstore/accountdiffs.go @@ -82,20 +82,34 @@ func (b *AccountDiffs) Delete(accountID iotago.AccountID) (err error) { return b.diffChangeStore.Delete(accountID) } +func (b *AccountDiffs) Clear() error { + if err := b.diffChangeStore.Clear(); err != nil { + return err + } + + return b.destroyedAccounts.Clear() +} + // Stream streams all accountIDs changes for a slot index. func (b *AccountDiffs) Stream(consumer func(accountID iotago.AccountID, accountDiff *model.AccountDiff, destroyed bool) bool) error { - // We firstly iterate over the destroyed accounts, as they won't have a corresponding accountDiff. - if storageErr := b.destroyedAccounts.Iterate(kvstore.EmptyPrefix, func(accountID iotago.AccountID, _ types.Empty) bool { - return consumer(accountID, nil, true) - }); storageErr != nil { - return ierrors.Wrapf(storageErr, "failed to iterate over account diffs for slot %s", b.slot) - } + // Existing accounts modified in the slot only have a SlotDiff, but are not part of the b.destroyedAccount store. + // Destroyed accounts should also have a SlotDiff + // and be part of b.destroyedAccount so that it's possible to re-create those. - // For those accounts that still exist, we might have an accountDiff. + var internalErr error if storageErr := b.diffChangeStore.Iterate(kvstore.EmptyPrefix, func(accountID iotago.AccountID, accountDiff *model.AccountDiff) bool { - return consumer(accountID, accountDiff, false) + destroyed, err := b.destroyedAccounts.Has(accountID) + if err != nil { + internalErr = ierrors.Wrapf(err, "failed to check if an account %s was destroyed", accountID) + + return false + } + + return consumer(accountID, accountDiff, destroyed) }); storageErr != nil { return ierrors.Wrapf(storageErr, "failed to iterate over account diffs for slot %s", b.slot) + } else if internalErr != nil { + return internalErr } return nil