Skip to content

Commit 617b524

Browse files
marcos-iovmarcos-iov
authored
Merge pull request #3415 from rsksmart/blockexecutor-refactor
BlockExecutor refactor
2 parents 45c2b36 + fb25d6a commit 617b524

File tree

2 files changed

+50
-81
lines changed

2 files changed

+50
-81
lines changed

‎rskj-core/src/main/java/co/rsk/core/bc/BlockExecutor.java‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@
2828
import co.rsk.metrics.profilers.MetricKind;
2929
import co.rsk.metrics.profilers.Profiler;
3030
import co.rsk.metrics.profilers.ProfilerFactory;
31-
import co.rsk.peg.union.UnionBridgeStorageIndexKey;
31+
import co.rsk.peg.storage.BridgeStorageAccessorImpl;
32+
import co.rsk.peg.storage.StorageAccessor;
33+
import co.rsk.peg.union.UnionBridgeStorageProvider;
34+
import co.rsk.peg.union.UnionBridgeStorageProviderImpl;
3235
import com.google.common.annotations.VisibleForTesting;
3336
import org.ethereum.config.Constants;
3437
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
@@ -76,7 +79,6 @@
7679

7780
import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP126;
7881
import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP85;
79-
import static org.ethereum.util.ByteUtil.EMPTY_BYTE_ARRAY;
8082
import static org.ethereum.util.ByteUtil.toHexString;
8183

8284
/**
@@ -200,30 +202,28 @@ private void fill(Block block, BlockResult result) {
200202
header.setPaidFees(result.getPaidFees());
201203
header.setLogsBloom(calculateLogsBloom(result.getTransactionReceipts()));
202204
header.setTxExecutionSublistsEdges(result.getTxEdges());
203-
setBaseEventIfRskip535IsActive(block, header);
205+
setBaseEvent(header);
204206

205207
block.flushRLP();
206208
profiler.stop(metric);
207209
}
208210

209-
private void setBaseEventIfRskip535IsActive(Block block, BlockHeader header) {
210-
if (block != null && header != null && activationConfig.isActive(ConsensusRule.RSKIP535, block.getNumber())) {
211-
try {
212-
Repository repo = repositoryLocator.startTrackingAt(header);
213-
if (repo != null) {
214-
RskAddress address = PrecompiledContracts.BRIDGE_ADDR;
215-
DataWord key = UnionBridgeStorageIndexKey.BASE_EVENT.getKey();
216-
byte[] baseEvent = repo.getStorageBytes(address, key);
217-
if (baseEvent == null) {
218-
baseEvent = EMPTY_BYTE_ARRAY;
219-
}
220-
header.setBaseEvent(baseEvent);
221-
}
222-
} catch (Exception e) {
223-
// If repository access fails, just skip setting baseEvent
224-
// This can happen in test environments or when repository is not available
225-
logger.warn("Failed to set baseEvent in block header. {}", e.getMessage());
226-
}
211+
private void setBaseEvent(BlockHeader header) {
212+
if (header.getVersion() < 2) {
213+
return;
214+
}
215+
try {
216+
Repository repo = repositoryLocator.startTrackingAt(header);
217+
StorageAccessor bridgeStorageAccessor = new BridgeStorageAccessorImpl(repo);
218+
UnionBridgeStorageProvider unionBridgeStorageProvider = new UnionBridgeStorageProviderImpl(bridgeStorageAccessor);
219+
byte[] baseEvent = unionBridgeStorageProvider.getBaseEvent();
220+
221+
header.setBaseEvent(baseEvent);
222+
} catch (IllegalArgumentException e) {
223+
// If repository access fails, just skip setting baseEvent
224+
// This can happen in test environments or when repository is not available
225+
logger.warn("Failed to set baseEvent in block header. {}", e.getMessage());
226+
// TODO: Double check why this exception might be thrown and what the expected behaviour should be
227227
}
228228
}
229229

‎rskj-core/src/test/java/co/rsk/core/bc/BlockExecutorTest.java‎

Lines changed: 29 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,13 @@
100100
import static org.mockito.Mockito.doReturn;
101101
import static org.mockito.Mockito.spy;
102102

103-
/**
104-
* Created by ajlopez on 29/07/2016.
105-
*/
106103
public class BlockExecutorTest {
107104
private static final byte[] EMPTY_TRIE_HASH = sha3(RLP.encodeElement(EMPTY_BYTE_ARRAY));
108105
private static final boolean RSKIP_126_IS_ACTIVE = true;
109106
private static final long MIN_SEQUENTIAL_SET_GAS_LIMIT = Constants.regtest().getMinSequentialSetGasLimit();
110107

111108
private final TestSystemProperties config = new TestSystemProperties();
112109
private final ActivationConfig activationConfig = spy(config.getActivationConfig());
113-
private final BlockFactory BLOCK_FACTORY = new BlockFactory(activationConfig);
114110

115111
@TempDir
116112
public Path tempDir;
@@ -171,7 +167,7 @@ private static byte[] sha3(byte[] input) {
171167
}
172168

173169
@BeforeEach
174-
public void setUp() {
170+
void setUp() {
175171
RskTestFactory objects = new RskTestFactory(tempDir, config);
176172
blockchain = objects.getBlockchain();
177173
trieStore = objects.getTrieStore();
@@ -1484,6 +1480,7 @@ private BlockExecutor buildBlockExecutor(TrieStore store, RskSystemProperties co
14841480
doReturn(activeRskip144).when(activationConfig).isActive(eq(RSKIP144), anyLong());
14851481
doReturn(activeRskip126).when(activationConfig).isActive(eq(RSKIP126), anyLong());
14861482

1483+
BlockFactory blockFactory = new BlockFactory(activationConfig);
14871484

14881485
StateRootHandler stateRootHandler = new StateRootHandler(cfg.getActivationConfig(), new StateRootsStoreImpl(new HashMapDB()));
14891486

@@ -1496,17 +1493,18 @@ private BlockExecutor buildBlockExecutor(TrieStore store, RskSystemProperties co
14961493
BlockTxSignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache());
14971494

14981495
return new BlockExecutor(
1499-
new RepositoryLocator(store, stateRootHandler),
1500-
new TransactionExecutorFactory(
1501-
cfg,
1502-
null,
1503-
null,
1504-
BLOCK_FACTORY,
1505-
new ProgramInvokeFactoryImpl(),
1506-
new PrecompiledContracts(cfg, bridgeSupportFactory, signatureCache),
1507-
signatureCache
1508-
),
1509-
cfg);
1496+
new RepositoryLocator(store, stateRootHandler),
1497+
new TransactionExecutorFactory(
1498+
cfg,
1499+
null,
1500+
null,
1501+
blockFactory,
1502+
new ProgramInvokeFactoryImpl(),
1503+
new PrecompiledContracts(cfg, bridgeSupportFactory, signatureCache),
1504+
signatureCache
1505+
),
1506+
cfg
1507+
);
15101508
}
15111509

15121510
@ParameterizedTest
@@ -1639,18 +1637,19 @@ void executeBlockWithBaseEventFromRepository(boolean activeRskip535) {
16391637
Assertions.assertInstanceOf(BlockHeaderV2.class, block.getHeader(), "Block should have BlockHeaderV2 header when RSKIP535 is active");
16401638

16411639
// Create a real repository with the expected baseEvent value
1642-
byte[] expectedBaseEvent = new byte[]{0x01, 0x02, 0x03, 0x04, 0x05};
1643-
Repository repository = new MutableRepository(trieStore, new Trie(trieStore));
1644-
repository.startTracking();
1640+
byte[] baseEvent = new byte[128];
1641+
Arrays.fill(baseEvent, (byte) 0xFF);
1642+
Repository mutableRepository = new MutableRepository(trieStore, new Trie(trieStore));
1643+
mutableRepository.startTracking();
16451644

16461645
// Set the baseEvent in the repository at the bridge address
16471646
RskAddress bridgeAddress = PrecompiledContracts.BRIDGE_ADDR;
16481647
DataWord baseEventKey = UnionBridgeStorageIndexKey.BASE_EVENT.getKey();
1649-
repository.addStorageBytes(bridgeAddress, baseEventKey, expectedBaseEvent);
1650-
repository.commit();
1648+
mutableRepository.addStorageBytes(bridgeAddress, baseEventKey, baseEvent);
1649+
mutableRepository.commit();
16511650

16521651
// Update the parent block's state root to include our repository changes
1653-
parent.setStateRoot(repository.getRoot());
1652+
parent.setStateRoot(mutableRepository.getRoot());
16541653

16551654
// when
16561655
BlockExecutor executor = buildBlockExecutor(trieStore, true, RSKIP_126_IS_ACTIVE);
@@ -1659,7 +1658,7 @@ void executeBlockWithBaseEventFromRepository(boolean activeRskip535) {
16591658
BlockResult result = executor.executeAndFill(block, parent.getHeader());
16601659
Assertions.assertNotNull(result);
16611660
Assertions.assertNotSame(BlockResult.INTERRUPTED_EXECUTION_BLOCK_RESULT, result);
1662-
Assertions.assertArrayEquals(expectedBaseEvent, result.getBlock().getBaseEvent(),
1661+
Assertions.assertArrayEquals(baseEvent, result.getBlock().getBaseEvent(),
16631662
"BaseEvent should be set from repository when RSKIP535 is active");
16641663
} else {
16651664
// when RSKIP535 is not active, we should have a different header type
@@ -1677,17 +1676,19 @@ void executeBlockWithBaseEventFromRepository(boolean activeRskip535) {
16771676
}
16781677

16791678
@Test
1680-
void executeBlockWithBaseEventFromRepository_WhenRepositoryIsNull() {
1679+
void executeBlockWithBaseEventFromRepository_WhenNoBaseEventIsSet_ShouldNotFail() {
16811680
// given
1681+
doReturn(true).when(activationConfig).isActive(eq(ConsensusRule.RSKIP535), anyLong());
1682+
16821683
// Create a block with BlockHeaderV2
16831684
Block parent = blockchain.getBestBlock();
16841685
Block block = new BlockGenerator(Constants.regtest(), activationConfig).createChildBlock(parent);
16851686

16861687
// Create a real repository but don't set any baseEvent
1687-
Repository repository = new MutableRepository(trieStore, new Trie(trieStore));
1688-
repository.startTracking();
1689-
repository.commit();
1690-
parent.setStateRoot(repository.getRoot());
1688+
Repository mutableRepository = new MutableRepository(trieStore, new Trie(trieStore));
1689+
mutableRepository.startTracking();
1690+
mutableRepository.commit();
1691+
parent.setStateRoot(mutableRepository.getRoot());
16911692

16921693
BlockExecutor executor = buildBlockExecutor(trieStore, true, RSKIP_126_IS_ACTIVE);
16931694

@@ -1701,38 +1702,6 @@ void executeBlockWithBaseEventFromRepository_WhenRepositoryIsNull() {
17011702
"BaseEvent should be set to empty array when repository has no baseEvent");
17021703
}
17031704

1704-
@Test
1705-
void executeBlockWithBaseEventFromRepository_WhenRepositoryThrowsException() {
1706-
// given
1707-
doReturn(true).when(activationConfig).isActive(eq(ConsensusRule.RSKIP535), anyLong());
1708-
1709-
Block parent = blockchain.getBestBlock();
1710-
Block block = new BlockGenerator(Constants.regtest(), activationConfig).createChildBlock(parent);
1711-
1712-
// Create a real repository with baseEvent data
1713-
Repository repository = new MutableRepository(trieStore, new Trie(trieStore));
1714-
repository.startTracking();
1715-
RskAddress bridgeAddress = PrecompiledContracts.BRIDGE_ADDR;
1716-
DataWord baseEventKey = UnionBridgeStorageIndexKey.BASE_EVENT.getKey();
1717-
byte[] expectedBaseEvent = new byte[]{0x01, 0x02, 0x03, 0x04, 0x05};
1718-
repository.addStorageBytes(bridgeAddress, baseEventKey, expectedBaseEvent);
1719-
repository.commit();
1720-
parent.setStateRoot(repository.getRoot());
1721-
1722-
// Create BlockExecutor with real repository
1723-
BlockExecutor executor = buildBlockExecutor(trieStore, true, RSKIP_126_IS_ACTIVE);
1724-
1725-
// when
1726-
BlockResult result = executor.executeAndFill(block, parent.getHeader());
1727-
1728-
// then
1729-
Assertions.assertNotNull(result);
1730-
Assertions.assertNotSame(BlockResult.INTERRUPTED_EXECUTION_BLOCK_RESULT, result);
1731-
// When repository has baseEvent, it should be retrieved correctly
1732-
Assertions.assertArrayEquals(expectedBaseEvent, result.getBlock().getBaseEvent(),
1733-
"BaseEvent should be retrieved from repository when available");
1734-
}
1735-
17361705
public static class TestObjects {
17371706
private final TrieStore trieStore;
17381707
private final Block block;

0 commit comments

Comments
 (0)