diff --git a/actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java b/actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java index 9de7c0691ba..39e2888b776 100644 --- a/actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java +++ b/actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java @@ -900,7 +900,10 @@ private long increase(long lastUsage, long usage, long lastTime, long now, long long averageUsage = divideCeil(usage * precision, windowSize); if (lastTime != now) { - assert now > lastTime; + if (now < lastTime) { + throw new IllegalArgumentException( + "Time went backwards: now=" + now + ", lastTime=" + lastTime); + } if (lastTime + windowSize > now) { long delta = now - lastTime; double decay = (windowSize - delta) / (double) windowSize; @@ -930,7 +933,10 @@ public long calculateGlobalEnergyLimit(AccountCapsule accountCapsule) { long totalEnergyLimit = getDynamicPropertiesStore().getTotalEnergyCurrentLimit(); long totalEnergyWeight = getDynamicPropertiesStore().getTotalEnergyWeight(); - assert totalEnergyWeight > 0; + if (totalEnergyWeight <= 0) { + throw new IllegalStateException( + "totalEnergyWeight must be positive, got: " + totalEnergyWeight); + } return (long) (energyWeight * ((double) totalEnergyLimit / totalEnergyWeight)); } diff --git a/chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java b/chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java index 96d6ceac893..95f6af5953c 100644 --- a/chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java +++ b/chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java @@ -75,7 +75,10 @@ private static long convertVectorToLong(List v) throws ZksnarkException } public byte[] encode() throws ZksnarkException { - assert (authenticationPath.size() == index.size()); + if (authenticationPath.size() != index.size()) { + throw new ZksnarkException("authenticationPath and index size mismatch: " + + authenticationPath.size() + " != " + index.size()); + } List> pathByteList = Lists.newArrayList(); long indexLong; // 64 for (int i = 0; i < authenticationPath.size(); i++) { diff --git a/chainbase/src/main/java/org/tron/core/db/EnergyProcessor.java b/chainbase/src/main/java/org/tron/core/db/EnergyProcessor.java index 30d778d0990..60bc271dc91 100644 --- a/chainbase/src/main/java/org/tron/core/db/EnergyProcessor.java +++ b/chainbase/src/main/java/org/tron/core/db/EnergyProcessor.java @@ -153,7 +153,10 @@ public long calculateGlobalEnergyLimit(AccountCapsule accountCapsule) { if (dynamicPropertiesStore.allowNewReward() && totalEnergyWeight <= 0) { return 0; } else { - assert totalEnergyWeight > 0; + if (totalEnergyWeight <= 0) { + throw new IllegalStateException( + "totalEnergyWeight must be positive, got: " + totalEnergyWeight); + } } return (long) (energyWeight * ((double) totalEnergyLimit / totalEnergyWeight)); } diff --git a/chainbase/src/main/java/org/tron/core/db/ResourceProcessor.java b/chainbase/src/main/java/org/tron/core/db/ResourceProcessor.java index 7e170f9dab5..4613a4cd630 100644 --- a/chainbase/src/main/java/org/tron/core/db/ResourceProcessor.java +++ b/chainbase/src/main/java/org/tron/core/db/ResourceProcessor.java @@ -49,7 +49,10 @@ protected long increase(long lastUsage, long usage, long lastTime, long now, lon long averageUsage = divideCeil(usage * precision, windowSize); if (lastTime != now) { - assert now > lastTime; + if (now < lastTime) { + throw new IllegalArgumentException( + "Time went backwards: now=" + now + ", lastTime=" + lastTime); + } if (lastTime + windowSize > now) { long delta = now - lastTime; double decay = (windowSize - delta) / (double) windowSize; diff --git a/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java b/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java index c3776921244..cc71aa89e24 100644 --- a/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java +++ b/chainbase/src/main/java/org/tron/core/db/TransactionTrace.java @@ -300,6 +300,11 @@ private void resetAccountUsage(AccountCapsule accountCap, - (mergedUsage * mergedSize - usage * size); // If area merging happened during suicide, use the current window size long newSize = mergedSize == currentSize ? size : currentSize; + if (newSize == 0) { + accountCap.setEnergyUsage(0); + accountCap.setNewWindowSize(ENERGY, 0L); + return; + } // Calc new usage by fixed x-axes long newUsage = max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath()); // Reset account usage and window size @@ -317,6 +322,11 @@ private void resetAccountUsageV2(AccountCapsule accountCap, // If area merging happened during suicide, use the current window size long newSize = mergedSize == currentSize ? size : currentSize; long newSize2 = mergedSize == currentSize ? size2 : currentSize2; + if (newSize == 0) { + accountCap.setEnergyUsage(0); + accountCap.setNewWindowSizeV2(ENERGY, 0L); + return; + } // Calc new usage by fixed x-axes long newUsage = max(0, newArea / newSize, dynamicPropertiesStore.disableJavaLangMath()); // Reset account usage and window size diff --git a/chainbase/src/main/java/org/tron/core/db/TronDatabase.java b/chainbase/src/main/java/org/tron/core/db/TronDatabase.java index 40762568c82..d72874363e5 100644 --- a/chainbase/src/main/java/org/tron/core/db/TronDatabase.java +++ b/chainbase/src/main/java/org/tron/core/db/TronDatabase.java @@ -78,12 +78,15 @@ public void close() { logger.info("******** Begin to close {}. ********", getName()); try { writeOptions.close(); + } catch (Exception e) { + logger.warn("Failed to close writeOptions in {}.", getName(), e); + } + try { dbSource.closeDB(); } catch (Exception e) { - logger.warn("Failed to close {}.", getName(), e); - } finally { - logger.info("******** End to close {}. ********", getName()); + logger.warn("Failed to close dbSource in {}.", getName(), e); } + logger.info("******** End to close {}. ********", getName()); } public abstract void put(byte[] key, T item); diff --git a/chainbase/src/main/java/org/tron/core/db2/common/LevelDB.java b/chainbase/src/main/java/org/tron/core/db2/common/LevelDB.java index 5942bb7444c..bbf55995156 100644 --- a/chainbase/src/main/java/org/tron/core/db2/common/LevelDB.java +++ b/chainbase/src/main/java/org/tron/core/db2/common/LevelDB.java @@ -4,11 +4,13 @@ import java.util.HashMap; import java.util.Map; import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.tron.common.parameter.CommonParameter; import org.tron.common.storage.WriteOptionsWrapper; import org.tron.common.storage.leveldb.LevelDbDataSourceImpl; import org.tron.core.db.common.iterator.DBIterator; +@Slf4j(topic = "DB") public class LevelDB implements DB, Flusher { @Getter @@ -65,8 +67,16 @@ public void flush(Map batch) { @Override public void close() { - this.writeOptions.close(); - db.closeDB(); + try { + this.writeOptions.close(); + } catch (Exception e) { + logger.warn("Failed to close writeOptions.", e); + } + try { + db.closeDB(); + } catch (Exception e) { + logger.warn("Failed to close db.", e); + } } @Override diff --git a/chainbase/src/main/java/org/tron/core/db2/common/RocksDB.java b/chainbase/src/main/java/org/tron/core/db2/common/RocksDB.java index 1d67438eceb..15828028fdd 100644 --- a/chainbase/src/main/java/org/tron/core/db2/common/RocksDB.java +++ b/chainbase/src/main/java/org/tron/core/db2/common/RocksDB.java @@ -4,11 +4,13 @@ import java.util.HashMap; import java.util.Map; import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.tron.common.parameter.CommonParameter; import org.tron.common.storage.WriteOptionsWrapper; import org.tron.common.storage.rocksdb.RocksDbDataSourceImpl; import org.tron.core.db.common.iterator.DBIterator; +@Slf4j(topic = "DB") public class RocksDB implements DB, Flusher { @Getter @@ -66,8 +68,16 @@ public void flush(Map batch) { @Override public void close() { - writeOptions.close(); - db.closeDB(); + try { + writeOptions.close(); + } catch (Exception e) { + logger.warn("Failed to close writeOptions.", e); + } + try { + db.closeDB(); + } catch (Exception e) { + logger.warn("Failed to close db.", e); + } } @Override diff --git a/chainbase/src/main/java/org/tron/core/db2/core/Chainbase.java b/chainbase/src/main/java/org/tron/core/db2/core/Chainbase.java index 17a047f78ae..1373173f1bd 100644 --- a/chainbase/src/main/java/org/tron/core/db2/core/Chainbase.java +++ b/chainbase/src/main/java/org/tron/core/db2/core/Chainbase.java @@ -35,7 +35,7 @@ public enum Cursor { //true:fullnode, false:soliditynode private ThreadLocal cursor = new ThreadLocal<>(); private ThreadLocal offset = new ThreadLocal<>(); - private Snapshot head; + private volatile Snapshot head; public Chainbase(Snapshot head) { this.head = head; @@ -349,28 +349,32 @@ private Map getNext(Snapshot head, byte[] key, long limit) { .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } + // Snapshot head once: prefixQueryRoot and prefixQuerySnapshot both need the same + // head reference. Other read methods (get, getUnchecked, has) only read head once + // via head(), so volatile alone is sufficient for them. public Map prefixQuery(byte[] key) { - Map result = prefixQueryRoot(key); - Map snapshot = prefixQuerySnapshot(key); + Snapshot localHead = head; + Map result = prefixQueryRoot(localHead, key); + Map snapshot = prefixQuerySnapshot(localHead, key); result.putAll(snapshot); result.entrySet().removeIf(e -> e.getValue() == null); return result; } - private Map prefixQueryRoot(byte[] key) { + private Map prefixQueryRoot(Snapshot localHead, byte[] key) { Map result = new HashMap<>(); - if (((SnapshotRoot) head.getRoot()).db.getClass() == LevelDB.class) { - result = ((LevelDB) ((SnapshotRoot) head.getRoot()).db).getDb().prefixQuery(key); - } else if (((SnapshotRoot) head.getRoot()).db.getClass() == RocksDB.class) { - result = ((RocksDB) ((SnapshotRoot) head.getRoot()).db).getDb().prefixQuery(key); + if (((SnapshotRoot) localHead.getRoot()).db.getClass() == LevelDB.class) { + result = ((LevelDB) ((SnapshotRoot) localHead.getRoot()).db).getDb().prefixQuery(key); + } else if (((SnapshotRoot) localHead.getRoot()).db.getClass() == RocksDB.class) { + result = ((RocksDB) ((SnapshotRoot) localHead.getRoot()).db).getDb().prefixQuery(key); } return result; } - private Map prefixQuerySnapshot(byte[] key) { + private Map prefixQuerySnapshot(Snapshot localHead, byte[] key) { Map result = new HashMap<>(); - Snapshot snapshot = head(); - if (!snapshot.equals(head.getRoot())) { + Snapshot snapshot = localHead; + if (!snapshot.equals(localHead.getRoot())) { Map all = new HashMap<>(); ((SnapshotImpl) snapshot).collect(all, key); all.forEach((k, v) -> result.put(k, v.getBytes())); diff --git a/chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java b/chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java index e20490d93c0..c230b4ac422 100644 --- a/chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java +++ b/chainbase/src/main/java/org/tron/core/db2/core/SnapshotManager.java @@ -511,9 +511,9 @@ private void checkV2() { if (latestTimestamp - timestamp > ONE_MINUTE_MILLS*2) { continue; } - TronDatabase checkPointV2Store = getCheckpointDB(cp); - recover(checkPointV2Store); - checkPointV2Store.close(); + try (TronDatabase checkPointV2Store = getCheckpointDB(cp)) { + recover(checkPointV2Store); + } } logger.info("checkpoint v2 recover success"); unChecked = false; diff --git a/chainbase/src/main/java/org/tron/core/store/CheckPointV2Store.java b/chainbase/src/main/java/org/tron/core/store/CheckPointV2Store.java index f027bd02664..373daa4dffa 100644 --- a/chainbase/src/main/java/org/tron/core/store/CheckPointV2Store.java +++ b/chainbase/src/main/java/org/tron/core/store/CheckPointV2Store.java @@ -63,19 +63,19 @@ public void updateByBatch(Map rows) { } /** - * close the database. + * Closes the database and releases all resources. + * Note: this class and the parent TronDatabase each declare their own + * writeOptions field (Java fields are not polymorphic). Both must be + * closed: this.writeOptions here, and the parent's via super.close(). */ @Override public void close() { - logger.debug("******** Begin to close {}. ********", getName()); try { writeOptions.close(); - dbSource.closeDB(); } catch (Exception e) { - logger.warn("Failed to close {}.", getName(), e); - } finally { - logger.debug("******** End to close {}. ********", getName()); + logger.warn("Failed to close writeOptions in {}.", getName(), e); } + super.close(); } } diff --git a/framework/src/main/java/org/tron/core/net/service/effective/ResilienceService.java b/framework/src/main/java/org/tron/core/net/service/effective/ResilienceService.java index b99b5b52bad..9ead13f86e4 100644 --- a/framework/src/main/java/org/tron/core/net/service/effective/ResilienceService.java +++ b/framework/src/main/java/org/tron/core/net/service/effective/ResilienceService.java @@ -8,8 +8,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Random; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -114,7 +114,7 @@ private void disconnectRandom() { .collect(Collectors.toList()); } if (!peers.isEmpty()) { - int index = new Random().nextInt(peers.size()); + int index = ThreadLocalRandom.current().nextInt(peers.size()); disconnectFromPeer(peers.get(index), ReasonCode.RANDOM_ELIMINATION, DisconnectCause.RANDOM_ELIMINATION); } @@ -236,11 +236,9 @@ private enum DisconnectCause { static class WeightedRandom { private final Map weights; - private final Random random; public WeightedRandom(Map weights) { this.weights = weights; - this.random = new Random(); } public Object next() { @@ -248,7 +246,11 @@ public Object next() { for (int weight : weights.values()) { totalWeight += weight; } - int randomNum = random.nextInt(totalWeight); + if (totalWeight <= 0) { + throw new IllegalStateException( + "Total weight must be positive, got: " + totalWeight); + } + int randomNum = ThreadLocalRandom.current().nextInt(totalWeight); for (Object key : weights.keySet()) { randomNum -= weights.get(key); if (randomNum < 0) { diff --git a/framework/src/test/java/org/tron/common/zksnark/MerklePathEncodeTest.java b/framework/src/test/java/org/tron/common/zksnark/MerklePathEncodeTest.java new file mode 100644 index 00000000000..b71fb0374e0 --- /dev/null +++ b/framework/src/test/java/org/tron/common/zksnark/MerklePathEncodeTest.java @@ -0,0 +1,32 @@ +package org.tron.common.zksnark; + +import static org.junit.Assert.assertThrows; + +import com.google.common.collect.Lists; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.Test; +import org.tron.core.exception.ZksnarkException; + +public class MerklePathEncodeTest { + + @Test + public void testEncode_sizeMismatch_throwsZksnarkException() { + List> authPath = Arrays.asList( + Arrays.asList(true, false), + Arrays.asList(false, true) + ); + List index = Collections.singletonList(true); // size 1, authPath size 2 + + MerklePath merklePath = new MerklePath(authPath, index); + + assertThrows(ZksnarkException.class, merklePath::encode); + } + + @Test + public void testEncode_emptyLists_noException() throws ZksnarkException { + MerklePath merklePath = new MerklePath(Lists.newArrayList(), Lists.newArrayList()); + merklePath.encode(); // should not throw + } +} diff --git a/framework/src/test/java/org/tron/core/db/DefensiveChecksTest.java b/framework/src/test/java/org/tron/core/db/DefensiveChecksTest.java new file mode 100644 index 00000000000..2859060eab1 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/DefensiveChecksTest.java @@ -0,0 +1,121 @@ +package org.tron.core.db; + +import static org.junit.Assert.assertThrows; + +import com.google.protobuf.ByteString; +import org.junit.Test; +import org.tron.common.BaseTest; +import org.tron.common.TestConstants; +import org.tron.common.utils.ByteArray; +import org.tron.core.capsule.AccountCapsule; +import org.tron.core.config.args.Args; +import org.tron.core.store.StoreFactory; +import org.tron.core.vm.repository.RepositoryImpl; +import org.tron.protos.Protocol.AccountType; + +/** + * Verifies that assert statements replaced with explicit if-throw checks + * in PR-1 (Commit 1) correctly throw exceptions when invariants are violated. + */ +public class DefensiveChecksTest extends BaseTest { + + static { + Args.setParam(new String[]{"--output-directory", dbPath()}, TestConstants.TEST_CONF); + } + + // ------------------------------------------------------------------------- + // EnergyProcessor.calculateGlobalEnergyLimit() + // ------------------------------------------------------------------------- + + @Test + public void testEnergyProcessorZeroWeightThrowsWhenNewRewardDisabled() { + // Arrange: new-reward feature off, energy weight = 0, account with enough frozen balance + dbManager.getDynamicPropertiesStore().saveUnfreezeDelayDays(0L); + dbManager.getDynamicPropertiesStore().saveAllowNewReward(0L); + dbManager.getDynamicPropertiesStore().saveTotalEnergyWeight(0L); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("test"), + ByteString.copyFrom(ByteArray.fromHexString( + "548794500882809695a8a687866e76d4271a1abc")), + AccountType.Normal, + 0L); + account.setFrozenForEnergy(1_000_000L, 0L); + + EnergyProcessor processor = new EnergyProcessor( + dbManager.getDynamicPropertiesStore(), dbManager.getAccountStore()); + + assertThrows(IllegalStateException.class, + () -> processor.calculateGlobalEnergyLimit(account)); + } + + @Test + public void testEnergyProcessorZeroWeightReturnsZeroWhenNewRewardEnabled() { + // When allowNewReward is on, totalEnergyWeight == 0 should return 0 (not throw) + dbManager.getDynamicPropertiesStore().saveUnfreezeDelayDays(0L); + dbManager.getDynamicPropertiesStore().saveAllowNewReward(1L); + dbManager.getDynamicPropertiesStore().saveTotalEnergyWeight(0L); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("test"), + ByteString.copyFrom(ByteArray.fromHexString( + "548794500882809695a8a687866e76d4271a1abc")), + AccountType.Normal, + 0L); + account.setFrozenForEnergy(1_000_000L, 0L); + + EnergyProcessor processor = new EnergyProcessor( + dbManager.getDynamicPropertiesStore(), dbManager.getAccountStore()); + + long result = processor.calculateGlobalEnergyLimit(account); + org.junit.Assert.assertEquals(0L, result); + } + + // ------------------------------------------------------------------------- + // ResourceProcessor.increase() — tested via EnergyProcessor.useEnergy() + // ------------------------------------------------------------------------- + + @Test + public void testResourceProcessor_increase_timeBackwards_throwsIllegalArgument() { + // Arrange: no freeze delay (so old increase() path is used), account with + // latestConsumeTime = 1000 and caller supplies now = 500 (< lastTime) + dbManager.getDynamicPropertiesStore().saveUnfreezeDelayDays(0L); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("test2"), + ByteString.copyFrom(ByteArray.fromHexString( + "abd4b9367799eaa3197fecb144eb71de1e049abc")), + AccountType.Normal, + 0L); + account.setLatestConsumeTimeForEnergy(1000L); + // frozeBalance = 0 < TRX_PRECISION → calculateGlobalEnergyLimit() returns 0 safely + + EnergyProcessor processor = new EnergyProcessor( + dbManager.getDynamicPropertiesStore(), dbManager.getAccountStore()); + + assertThrows(IllegalArgumentException.class, + () -> processor.useEnergy(account, 100L, 500L)); + } + + // ------------------------------------------------------------------------- + // RepositoryImpl.calculateGlobalEnergyLimit() + // ------------------------------------------------------------------------- + + @Test + public void testRepositoryImpl_calculateGlobalEnergyLimit_zeroWeight_throws() { + dbManager.getDynamicPropertiesStore().saveTotalEnergyWeight(0L); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("test3"), + ByteString.copyFrom(ByteArray.fromHexString( + "548794500882809695a8a687866e76d4271a1abc")), + AccountType.Normal, + 0L); + account.setFrozenForEnergy(1_000_000L, 0L); + + RepositoryImpl repository = RepositoryImpl.createRoot(StoreFactory.getInstance()); + + assertThrows(IllegalStateException.class, + () -> repository.calculateGlobalEnergyLimit(account)); + } +} diff --git a/framework/src/test/java/org/tron/core/db/ResourceCloseTest.java b/framework/src/test/java/org/tron/core/db/ResourceCloseTest.java new file mode 100644 index 00000000000..d77b3946f91 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/ResourceCloseTest.java @@ -0,0 +1,86 @@ +package org.tron.core.db; + +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import java.lang.reflect.Field; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.tron.common.TestConstants; +import org.tron.common.storage.WriteOptionsWrapper; +import org.tron.core.config.args.Args; +import org.tron.core.db.common.DbSourceInter; +import org.tron.core.store.CheckPointV2Store; + +/** + * Verifies that close() methods properly release all resources even when + * one resource's close() throws an exception. + */ +public class ResourceCloseTest { + + @ClassRule + public static final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + static { + org.rocksdb.RocksDB.loadLibrary(); + } + + @BeforeClass + public static void init() throws Exception { + Args.setParam( + new String[]{"-d", temporaryFolder.newFolder().toString()}, + TestConstants.TEST_CONF); + } + + @AfterClass + public static void destroy() { + Args.clearParam(); + } + + @Test + public void testTronDatabase_closeDbSource_whenWriteOptionsThrows() throws Exception { + CheckPointV2Store store = new CheckPointV2Store("test-close-safety"); + + // Get parent class fields via reflection + Field writeOptionsField = TronDatabase.class.getDeclaredField("writeOptions"); + writeOptionsField.setAccessible(true); + WriteOptionsWrapper originalWriteOptions = + (WriteOptionsWrapper) writeOptionsField.get(store); + + Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource"); + dbSourceField.setAccessible(true); + DbSourceInter originalDbSource = + (DbSourceInter) dbSourceField.get(store); + + // Replace with spies + WriteOptionsWrapper spyWriteOptions = spy(originalWriteOptions); + doThrow(new RuntimeException("writeOptions close failed")) + .when(spyWriteOptions).close(); + writeOptionsField.set(store, spyWriteOptions); + + DbSourceInter spyDbSource = spy(originalDbSource); + dbSourceField.set(store, spyDbSource); + + // Also spy the child's writeOptions + Field childWriteOptionsField = CheckPointV2Store.class.getDeclaredField("writeOptions"); + childWriteOptionsField.setAccessible(true); + WriteOptionsWrapper childOriginal = + (WriteOptionsWrapper) childWriteOptionsField.get(store); + WriteOptionsWrapper spyChildWriteOptions = spy(childOriginal); + doThrow(new RuntimeException("child writeOptions close failed")) + .when(spyChildWriteOptions).close(); + childWriteOptionsField.set(store, spyChildWriteOptions); + + // close() should not throw, and dbSource should still be closed + store.close(); + + verify(spyChildWriteOptions).close(); + verify(spyWriteOptions).close(); + verify(spyDbSource).closeDB(); + } +} diff --git a/framework/src/test/java/org/tron/core/db/TransactionTraceTest.java b/framework/src/test/java/org/tron/core/db/TransactionTraceTest.java index 08848fc9da1..9cacf3a06d1 100644 --- a/framework/src/test/java/org/tron/core/db/TransactionTraceTest.java +++ b/framework/src/test/java/org/tron/core/db/TransactionTraceTest.java @@ -18,6 +18,7 @@ import com.google.protobuf.Any; import com.google.protobuf.ByteString; import com.google.protobuf.InvalidProtocolBufferException; +import java.lang.reflect.Method; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -44,6 +45,7 @@ import org.tron.protos.Protocol.Transaction.Contract; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.Protocol.Transaction.raw; +import org.tron.protos.contract.Common.ResourceCode; import org.tron.protos.contract.SmartContractOuterClass.CreateSmartContract; import org.tron.protos.contract.SmartContractOuterClass.SmartContract; import org.tron.protos.contract.SmartContractOuterClass.TriggerSmartContract; @@ -446,4 +448,76 @@ public void testTriggerUseUsageInWindowSizeV2() throws VMIllegalException, Contr dbManager.getDynamicPropertiesStore().saveUnfreezeDelayDays(0); dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(0); } + + @Test + public void testResetAccountUsage_zeroNewSize() throws Exception { + // Ensure V1 path (not V2) + dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(0); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("testZeroSize"), + ByteString.copyFrom(ByteArray.fromInt(99)), + AccountType.Normal, + 0L); + // currentSize = 100, currentUsage = 500 + account.setNewWindowSize(ResourceCode.ENERGY, 100L); + account.setEnergyUsage(500L); + + // Build a minimal TransactionTrace to access the private method + Transaction trxn = Transaction.newBuilder().setRawData( + raw.newBuilder().addContract( + Contract.newBuilder().setType(ContractType.TransferContract))).build(); + TransactionCapsule trxCap = new TransactionCapsule(trxn); + TransactionTrace trace = new TransactionTrace(trxCap, StoreFactory.getInstance(), + new RuntimeImpl()); + + // Use reflection to call private resetAccountUsage + Method method = TransactionTrace.class.getDeclaredMethod("resetAccountUsage", + AccountCapsule.class, long.class, long.class, long.class, long.class, long.class); + method.setAccessible(true); + + // mergedSize == currentSize (100) and size == 0 → newSize = 0 + method.invoke(trace, account, /*usage=*/0L, /*size=*/0L, + /*mergedUsage=*/0L, /*mergedSize=*/100L, /*size2=*/0L); + + // energyUsage should be reset to 0 by the newSize == 0 guard + Assert.assertEquals(0L, account.getEnergyUsage()); + // windowSize internal value is 0, but getWindowSize() returns default 28800 when 0 + Assert.assertEquals(0L, + account.getInstance().getAccountResource().getEnergyWindowSize()); + } + + @Test + public void testResetAccountUsageV2_zeroNewSize() throws Exception { + // Ensure V2 path + dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(1); + + AccountCapsule account = new AccountCapsule( + ByteString.copyFromUtf8("testZeroSizeV2"), + ByteString.copyFrom(ByteArray.fromInt(98)), + AccountType.Normal, + 0L); + account.setNewWindowSize(ResourceCode.ENERGY, 100L); + account.setEnergyUsage(500L); + + Transaction trxn = Transaction.newBuilder().setRawData( + raw.newBuilder().addContract( + Contract.newBuilder().setType(ContractType.TransferContract))).build(); + TransactionCapsule trxCap = new TransactionCapsule(trxn); + TransactionTrace trace = new TransactionTrace(trxCap, StoreFactory.getInstance(), + new RuntimeImpl()); + + Method method = TransactionTrace.class.getDeclaredMethod("resetAccountUsage", + AccountCapsule.class, long.class, long.class, long.class, long.class, long.class); + method.setAccessible(true); + + // mergedSize == currentSize (100) and size == 0 → newSize = 0 + method.invoke(trace, account, /*usage=*/0L, /*size=*/0L, + /*mergedUsage=*/0L, /*mergedSize=*/100L, /*size2=*/0L); + + Assert.assertEquals(0L, account.getEnergyUsage()); + + // Restore + dbManager.getDynamicPropertiesStore().saveAllowCancelAllUnfreezeV2(0); + } } diff --git a/framework/src/test/java/org/tron/core/db2/ChainbaseConcurrentTest.java b/framework/src/test/java/org/tron/core/db2/ChainbaseConcurrentTest.java new file mode 100644 index 00000000000..eb13b850102 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db2/ChainbaseConcurrentTest.java @@ -0,0 +1,99 @@ +package org.tron.core.db2; + +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.Timeout; +import org.rocksdb.RocksDB; +import org.tron.common.BaseMethodTest; +import org.tron.common.storage.rocksdb.RocksDbDataSourceImpl; +import org.tron.core.config.args.Args; +import org.tron.core.db2.common.WrappedByteArray; +import org.tron.core.db2.core.Chainbase; +import org.tron.core.db2.core.SnapshotRoot; + +/** + * Verifies that concurrent setHead/prefixQuery operations on Chainbase + * do not throw exceptions after the volatile + local snapshot fix. + */ +public class ChainbaseConcurrentTest extends BaseMethodTest { + + @Rule + public Timeout globalTimeout = Timeout.seconds(60); + + @Override + protected void afterInit() { + RocksDB.loadLibrary(); + } + + @Test + public void testConcurrentSetHeadAndPrefixQuery() throws Exception { + RocksDbDataSourceImpl dataSource = new RocksDbDataSourceImpl( + Args.getInstance().getOutputDirectory(), "testConcurrentPrefixQuery"); + Chainbase chainbase = new Chainbase( + new SnapshotRoot(new org.tron.core.db2.common.RocksDB(dataSource))); + + try { + byte[] prefix = "test_prefix_".getBytes(); + + // Put initial data in root + chainbase.getHead().getRoot() + .put("test_prefix_key1".getBytes(), "value1".getBytes()); + chainbase.getHead().getRoot() + .put("test_prefix_key2".getBytes(), "value2".getBytes()); + + AtomicReference error = new AtomicReference<>(); + int iterations = 500; + CountDownLatch latch = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(4); + + // Reader threads: call prefixQuery concurrently + for (int t = 0; t < 3; t++) { + executor.submit(() -> { + try { + latch.await(); + for (int i = 0; i < iterations; i++) { + Map result = + chainbase.prefixQuery(prefix); + Assert.assertNotNull(result); + } + } catch (Throwable e) { + error.compareAndSet(null, e); + } + }); + } + + // Writer thread: advance head concurrently + executor.submit(() -> { + try { + latch.await(); + for (int i = 0; i < iterations; i++) { + chainbase.setHead(chainbase.getHead().advance()); + chainbase.getHead().put( + ("test_prefix_key_" + i).getBytes(), + ("value_" + i).getBytes()); + } + } catch (Throwable e) { + error.compareAndSet(null, e); + } + }); + + latch.countDown(); + executor.shutdown(); + executor.awaitTermination(30, TimeUnit.SECONDS); + + Assert.assertNull( + "Concurrent access caused an exception: " + error.get(), + error.get()); + } finally { + chainbase.reset(); + chainbase.close(); + } + } +} diff --git a/plugins/src/main/java/common/org/tron/plugins/DbConvert.java b/plugins/src/main/java/common/org/tron/plugins/DbConvert.java index bcf6e1e7afc..03dd940a440 100644 --- a/plugins/src/main/java/common/org/tron/plugins/DbConvert.java +++ b/plugins/src/main/java/common/org/tron/plugins/DbConvert.java @@ -3,6 +3,7 @@ import java.io.File; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -201,18 +202,15 @@ public String name() { return dbName; } - private void batchInsert(RocksDB rocks, List keys, List values) + private void batchInsert(RocksDB rocks, List> entries) throws Exception { try (org.rocksdb.WriteBatch batch = new org.rocksdb.WriteBatch()) { - for (int i = 0; i < keys.size(); i++) { - byte[] k = keys.get(i); - byte[] v = values.get(i); - batch.put(k, v); + for (Map.Entry entry : entries) { + batch.put(entry.getKey(), entry.getValue()); } write(rocks, batch); } - keys.clear(); - values.clear(); + entries.clear(); } /** @@ -254,8 +252,7 @@ private boolean maybeRetry(RocksDBException e) { * @return if ok */ public void convertLevelToRocks() throws Exception { - List keys = new ArrayList<>(BATCH); - List values = new ArrayList<>(BATCH); + List> entries = new ArrayList<>(BATCH); JniDBFactory.pushMemoryPool(1024 * 1024); try ( DB level = DBUtils.newLevelDb(srcDbPath); @@ -273,15 +270,14 @@ public void convertLevelToRocks() throws Exception { srcDbKeyCount++; srcDbKeySum = byteArrayToIntWithOne(srcDbKeySum, key); srcDbValueSum = byteArrayToIntWithOne(srcDbValueSum, value); - keys.add(key); - values.add(value); - if (keys.size() >= BATCH) { - batchInsert(rocks, keys, values); + entries.add(new AbstractMap.SimpleEntry<>(key, value)); + if (entries.size() >= BATCH) { + batchInsert(rocks, entries); } } // clear - if (!keys.isEmpty()) { - batchInsert(rocks, keys, values); + if (!entries.isEmpty()) { + batchInsert(rocks, entries); } } finally { JniDBFactory.popMemoryPool();