Skip to content

Commit

Permalink
fix: storage link migration and management (#13038)
Browse files Browse the repository at this point in the history 
Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
  • Loading branch information
@tinker-michaelj
tinker-michaelj committed Apr 29, 2024
1 parent 8227e78 commit c311db9
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 80 deletions.
7 changes: 2 additions & 5 deletions ...rc/main/java/com/hedera/node/app/service/mono/state/migration/AccountStateTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.mono.state.migration;

import static com.hedera.node.app.service.mono.state.migration.ContractStateMigrator.bytesFromInts;
import static java.util.Objects.requireNonNull;

import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -133,11 +134,7 @@ public static Account accountFromMerkle(
}

public static Account accountFromOnDiskAccount(@NonNull final OnDiskAccount account) {
final var firstContractStorageKey = account.getFirstContractStorageKey() == null
? Bytes.EMPTY
: Bytes.wrap(account.getFirstContractStorageKey()
.getKeyAsBigInteger()
.toByteArray());
final var firstContractStorageKey = bytesFromInts(account.getFirstStorageKey());
final var stakedAccountId = account.getStakedId() > 0
? AccountID.newBuilder().accountNum(account.getStakedId()).build()
: null;
Expand Down
138 changes: 70 additions & 68 deletions ...src/main/java/com/hedera/node/app/service/contract/impl/infra/IterableStorageManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.hedera.node.app.service.contract.impl.infra;

import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.tuweniToPbjBytes;
import static java.util.Objects.requireNonNull;

import com.hedera.hapi.node.base.ContractID;
import com.hedera.hapi.node.state.contract.SlotKey;
Expand All @@ -32,7 +33,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.apache.logging.log4j.LogManager;
Expand All @@ -59,7 +59,7 @@ public IterableStorageManager() {
*
* <p>Besides updating the first keys of these linked lists in the scoped accounts, also updates the
* slots used per contract via
* {@link HandleHederaOperations#updateStorageMetadata(long, Bytes, int)}.
* {@link HandleHederaOperations#updateStorageMetadata(ContractID, Bytes, int)}.
*
* @param enhancement the enhancement for the current transaction
* @param allAccesses the pending changes to storage values
Expand All @@ -77,19 +77,27 @@ public void persistChanges(
// Adjust the storage linked lists for each contract
allAccesses.forEach(contractAccesses -> contractAccesses.accesses().forEach(access -> {
if (access.isUpdate()) {
var firstContractKey = contractFirstKeyOf(enhancement, contractAccesses.contractID());

switch (StorageAccessType.getAccessType(access)) {
case REMOVAL -> firstContractKey = removeAccessedValue(
store, firstContractKey, contractAccesses.contractID(), tuweniToPbjBytes(access.key()));
case INSERTION -> firstContractKey = insertAccessedValue(
store,
firstContractKey,
tuweniToPbjBytes(access.writtenValue()),
contractAccesses.contractID(),
tuweniToPbjBytes(access.key()));
}
firstKeys.put(contractAccesses.contractID(), firstContractKey);
final var firstContractKey = contractFirstKeyOf(enhancement, contractAccesses.contractID());

// Only certain access types can change the head slot in a contract's storage linked list
final var newFirstContractKey =
switch (StorageAccessType.getAccessType(access)) {
case UNKNOWN, READ_ONLY, UPDATE -> firstContractKey;
// We might be removing the head slot from the existing list
case REMOVAL -> removeAccessedValue(
store,
firstContractKey,
contractAccesses.contractID(),
tuweniToPbjBytes(access.key()));
// We always insert the new slot at the head
case INSERTION -> insertAccessedValue(
store,
firstContractKey,
tuweniToPbjBytes(requireNonNull(access.writtenValue())),
contractAccesses.contractID(),
tuweniToPbjBytes(access.key()));
};
firstKeys.put(contractAccesses.contractID(), newFirstContractKey);
}
}));

Expand All @@ -108,16 +116,15 @@ public void persistChanges(

/**
* Returns the first storage key for the contract or Bytes.Empty if none exists.
*
* @param enhancement the enhancement for the current transaction
* @param contractNumber the contract number
* @param contractID the contract id
* @return the first storage key for the contract or null if none exists.
*/
@NonNull
private Bytes contractFirstKeyOf(@NonNull final Enhancement enhancement, ContractID contractID) {
private Bytes contractFirstKeyOf(@NonNull final Enhancement enhancement, @NonNull final ContractID contractID) {
final var account = enhancement.nativeOperations().getAccount(contractID);
return account != null && account.firstContractStorageKey() != null
? account.firstContractStorageKey()
: Bytes.EMPTY;
return account != null ? account.firstContractStorageKey() : Bytes.EMPTY;
}

/**
Expand All @@ -132,46 +139,34 @@ private Bytes contractFirstKeyOf(@NonNull final Enhancement enhancement, Contrac
@NonNull
private Bytes removeAccessedValue(
@NonNull final ContractStateStore store,
@NonNull final Bytes firstContractKey,
ContractID contractID,
@NonNull Bytes firstContractKey,
@NonNull final ContractID contractID,
@NonNull final Bytes key) {
requireNonNull(firstContractKey);
requireNonNull(contractID);
requireNonNull(store);
requireNonNull(key);
final var slotKey = new SlotKey(contractID, key);
try {
Objects.requireNonNull(store);
Objects.requireNonNull(key);
final var slotKey = newSlotKeyFor(contractID, key);
final var slotValue = slotValueFor(store, false, slotKey, "Missing key ");
final var nextKey = slotValue.nextKey();
final var prevKey = slotValue.previousKey();

if (!nextKey.equals(Bytes.EMPTY)) {
// Look up the next slot value
final var nextSlotKey = newSlotKeyFor(contractID, nextKey);
final var nextValue = slotValueFor(store, true, nextSlotKey, "Missing next key ");

// Create new next value and put into the store
final var newNextValue =
nextValue.copyBuilder().previousKey(prevKey).build();
store.putSlot(nextSlotKey, newNextValue);
if (!Bytes.EMPTY.equals(nextKey)) {
updatePrevFor(new SlotKey(contractID, nextKey), prevKey, store);
}
if (!prevKey.equals(Bytes.EMPTY)) {
// Look up the previous slot value
final var prevSlotKey = newSlotKeyFor(contractID, prevKey);
final var prevValue = slotValueFor(store, true, prevSlotKey, "Missing previous key ");

// Create new previous value and put into the store
final var newPrevValue =
prevValue.copyBuilder().nextKey(nextKey).build();
store.putSlot(prevSlotKey, newPrevValue);
if (!Bytes.EMPTY.equals(prevKey)) {
updateNextFor(new SlotKey(contractID, prevKey), nextKey, store);
}
store.removeSlot(slotKey);
return key.equals(firstContractKey) ? slotValue.nextKey() : firstContractKey;
firstContractKey = key.equals(firstContractKey) ? nextKey : firstContractKey;
} catch (Exception irreparable) {
// Since maintaining linked lists is not mission-critical, just log the error and continue
log.error(
"Failed link management when removing {}; will be unable to"
+ " expire all slots for this contract",
"Failed link management when removing {}; will be unable to" + " expire all slots for contract {}",
key,
contractID,
irreparable);
}
store.removeSlot(slotKey);
return firstContractKey;
}

Expand All @@ -190,30 +185,37 @@ private Bytes insertAccessedValue(
@NonNull final ContractStateStore store,
@NonNull final Bytes firstContractKey,
@NonNull final Bytes newValue,
ContractID contractID,
@NonNull final ContractID contractID,
@NonNull final Bytes newKey) {
requireNonNull(store);
requireNonNull(newKey);
requireNonNull(newValue);
try {
Objects.requireNonNull(store);
Objects.requireNonNull(newValue);
Objects.requireNonNull(newKey);
// Create new slot key and value and put into the store
final var newSlotKey = newSlotKeyFor(contractID, newKey);
final var newSlotValue = SlotValue.newBuilder()
.value(newValue)
.previousKey(Bytes.EMPTY)
.nextKey(firstContractKey)
.build();
store.putSlot(newSlotKey, newSlotValue);
return newKey;
if (!Bytes.EMPTY.equals(firstContractKey)) {
updatePrevFor(new SlotKey(contractID, firstContractKey), newKey, store);
}
} catch (Exception irreparable) {
log.error("Failed link management when inserting {}", newKey, irreparable);
// Since maintaining linked lists is not mission-critical, just log the error and continue
log.error(
"Failed link management when inserting {}; will be unable to" + " expire all slots for contract {}",
newKey,
contractID,
irreparable);
}
return firstContractKey;
store.putSlot(new SlotKey(contractID, newKey), new SlotValue(newValue, Bytes.EMPTY, firstContractKey));
return newKey;
}

@NonNull
private SlotKey newSlotKeyFor(ContractID contractNumber, @NonNull final Bytes key) {
return new SlotKey(contractNumber, key);
private void updatePrevFor(
@NonNull final SlotKey key, @NonNull final Bytes newPrevKey, @NonNull final ContractStateStore store) {
final var value = slotValueFor(store, true, key, "Missing next key ");
store.putSlot(key, value.copyBuilder().previousKey(newPrevKey).build());
}

private void updateNextFor(
@NonNull final SlotKey key, @NonNull final Bytes newNextKey, @NonNull final ContractStateStore store) {
final var value = slotValueFor(store, true, key, "Missing prev key ");
store.putSlot(key, value.copyBuilder().nextKey(newNextKey).build());
}

@NonNull
Expand All @@ -223,7 +225,7 @@ private SlotValue slotValueFor(
@NonNull final SlotKey slotKey,
@NonNull final String msgOnError) {
return forModify
? Objects.requireNonNull(store.getSlotValueForModify(slotKey), () -> msgOnError + slotKey.key())
: Objects.requireNonNull(store.getSlotValue(slotKey), () -> msgOnError + slotKey.key());
? requireNonNull(store.getSlotValueForModify(slotKey), () -> msgOnError + slotKey.key())
: requireNonNull(store.getSlotValue(slotKey), () -> msgOnError + slotKey.key());
}
}
75 changes: 68 additions & 7 deletions ...java/com/hedera/node/app/service/contract/impl/test/infra/IterableStorageManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.hedera.node.app.service.contract.impl.test.infra;

import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.tuweniToPbjBytes;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.never;
Expand All @@ -34,7 +35,6 @@
import com.hedera.node.app.service.contract.impl.state.StorageAccess;
import com.hedera.node.app.service.contract.impl.state.StorageAccesses;
import com.hedera.node.app.service.contract.impl.state.StorageSizeChange;
import com.hedera.node.app.service.contract.impl.utils.ConversionUtils;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import java.util.List;
import org.apache.tuweni.units.bigints.UInt256;
Expand All @@ -49,9 +49,9 @@ class IterableStorageManagerTest {
ContractID.newBuilder().contractNum(1L).build();
private final ContractID CONTRACT_2 =
ContractID.newBuilder().contractNum(2L).build();
private final Bytes BYTES_1 = ConversionUtils.tuweniToPbjBytes(UInt256.ONE);
private final Bytes BYTES_2 = ConversionUtils.tuweniToPbjBytes(UInt256.valueOf(2L));
private final Bytes BYTES_3 = ConversionUtils.tuweniToPbjBytes(UInt256.valueOf(3L));
private final Bytes BYTES_1 = tuweniToPbjBytes(UInt256.ONE);
private final Bytes BYTES_2 = tuweniToPbjBytes(UInt256.valueOf(2L));
private final Bytes BYTES_3 = tuweniToPbjBytes(UInt256.valueOf(3L));

@Mock
private HederaOperations hederaOperations;
Expand Down Expand Up @@ -151,6 +151,33 @@ void removeFirstSlot() {
verifyNoMoreInteractions(hederaOperations);
}

@Test
void stillRemovesSlotEvenIfNextSlotIsMissing() {
final var accesses = List.of(new StorageAccesses(
CONTRACT_1, List.of(StorageAccess.newWrite(UInt256.ONE, UInt256.MAX_VALUE, UInt256.ZERO))));

final var sizeChanges = List.of(new StorageSizeChange(CONTRACT_1, 1, 0));

given(enhancement.nativeOperations()).willReturn(hederaNativeOperations);
given(hederaNativeOperations.getAccount(CONTRACT_1)).willReturn(account);
given(account.firstContractStorageKey()).willReturn(BYTES_1);
given(enhancement.operations()).willReturn(hederaOperations);
// Deleting the first slot
given(store.getSlotValue(new SlotKey(CONTRACT_1, BYTES_1)))
.willReturn(new SlotValue(BYTES_1, Bytes.EMPTY, BYTES_2));
// The next slot is missing (invariant failure, should be impossible)
given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_2))).willReturn(null);

subject.persistChanges(enhancement, accesses, sizeChanges, store);

// Model deleting the first contract storage
verify(store).removeSlot(new SlotKey(CONTRACT_1, BYTES_1));
// The new first key is BYTES_2 as the first slot for the contract was deleted.
verify(hederaOperations).updateStorageMetadata(CONTRACT_1, BYTES_1, -1);
verifyNoMoreInteractions(store);
verifyNoMoreInteractions(hederaOperations);
}

@Test
void removeSecondSlot() {
final var accesses = List.of(new StorageAccesses(
Expand Down Expand Up @@ -199,7 +226,7 @@ void removeSlotValueNotFound() {

// The new first key is BYTES_1 as before running the test
verify(hederaOperations).updateStorageMetadata(CONTRACT_1, BYTES_1, -1);
verifyNoMoreInteractions(store);
verify(store).removeSlot(new SlotKey(CONTRACT_1, BYTES_2));
verifyNoMoreInteractions(hederaOperations);
}

Expand All @@ -222,7 +249,7 @@ void insertSlotIntoEmptyStorage() {
verify(store)
.putSlot(
new SlotKey(CONTRACT_1, BYTES_2),
new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, Bytes.EMPTY));
new SlotValue(tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, Bytes.EMPTY));

// The new first key is BYTES_2
verify(hederaOperations).updateStorageMetadata(CONTRACT_1, BYTES_2, 1);
Expand All @@ -241,6 +268,40 @@ void insertSlotIntoExistingStorage() {
given(hederaNativeOperations.getAccount(CONTRACT_1)).willReturn(account);
given(account.firstContractStorageKey()).willReturn(BYTES_1);
given(enhancement.operations()).willReturn(hederaOperations);
given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_1)))
.willReturn(new SlotValue(tuweniToPbjBytes(UInt256.ONE), Bytes.EMPTY, Bytes.EMPTY));

// Should insert into the head of the existing storage list
subject.persistChanges(enhancement, accesses, sizeChanges, store);

verify(store)
.putSlot(
new SlotKey(CONTRACT_1, BYTES_2),
new SlotValue(tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, BYTES_1));
verify(store)
.putSlot(
new SlotKey(CONTRACT_1, BYTES_1),
new SlotValue(tuweniToPbjBytes(UInt256.ONE), BYTES_2, Bytes.EMPTY));

// The new first key is BYTES_2
verify(hederaOperations).updateStorageMetadata(CONTRACT_1, BYTES_2, 1);
verifyNoMoreInteractions(store);
verifyNoMoreInteractions(hederaOperations);
}

@Test
void slotStillInsertedEvenWithMissingPointer() {
final var accesses = List.of(new StorageAccesses(
CONTRACT_1, List.of(StorageAccess.newWrite(UInt256.valueOf(2L), UInt256.ZERO, UInt256.MAX_VALUE))));

final var sizeChanges = List.of(new StorageSizeChange(CONTRACT_1, 0, 1));

given(enhancement.nativeOperations()).willReturn(hederaNativeOperations);
given(hederaNativeOperations.getAccount(CONTRACT_1)).willReturn(account);
given(account.firstContractStorageKey()).willReturn(BYTES_1);
given(enhancement.operations()).willReturn(hederaOperations);
// The next slot is missing (invariant failure, should be impossible)
given(store.getSlotValueForModify(new SlotKey(CONTRACT_1, BYTES_1))).willReturn(null);

// Insert into the second slot
subject.persistChanges(enhancement, accesses, sizeChanges, store);
Expand All @@ -249,7 +310,7 @@ void insertSlotIntoExistingStorage() {
verify(store)
.putSlot(
new SlotKey(CONTRACT_1, BYTES_2),
new SlotValue(ConversionUtils.tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, BYTES_1));
new SlotValue(tuweniToPbjBytes(UInt256.MAX_VALUE), Bytes.EMPTY, BYTES_1));

// The new first key is BYTES_2
verify(hederaOperations).updateStorageMetadata(CONTRACT_1, BYTES_2, 1);
Expand Down

0 comments on commit c311db9

Please sign in to comment.