-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(jsonrpc): post log/block filters for reorg-applied blocks #6819
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
base: release_v4.8.2
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,10 @@ | |
| import org.tron.common.TestConstants; | ||
| import org.tron.common.crypto.ECKey; | ||
| import org.tron.common.logsfilter.EventPluginLoader; | ||
| import org.tron.common.logsfilter.capsule.FilterTriggerCapsule; | ||
| import org.tron.common.logsfilter.capsule.LogsFilterCapsule; | ||
| import org.tron.common.logsfilter.trigger.ContractLogTrigger; | ||
| import org.tron.common.parameter.CommonParameter; | ||
| import org.tron.common.runtime.RuntimeImpl; | ||
| import org.tron.common.utils.ByteArray; | ||
| import org.tron.common.utils.Commons; | ||
|
|
@@ -1540,4 +1543,117 @@ public void adjustBalance(AccountStore accountStore, byte[] accountAddress, long | |
| Commons.adjustBalance(accountStore, accountAddress, amount, | ||
| chainManager.getDynamicPropertiesStore().disableJavaLangMath()); | ||
| } | ||
|
|
||
| /** | ||
| * Drives a real reorg and asserts what Manager posts to the jsonrpc filterCapsuleQueue. | ||
| */ | ||
| @Test | ||
| public void switchForkShouldPostFullNodeFilterForNewBranch() throws Exception { | ||
| CommonParameter.getInstance().jsonRpcHttpFullNodeEnable = true; | ||
| // filterProcessLoop only starts when isJsonRpcFilterEnabled() held at Manager.init() time; it | ||
| // was false then, so filterCapsuleQueue is produce-only here and fully observable. | ||
|
|
||
| // bootstrap a head with a known witness | ||
| String key = PublicMethod.getRandomPrivateKey(); | ||
| byte[] privateKey = ByteArray.fromHexString(key); | ||
| final ECKey ecKey = ECKey.fromPrivate(privateKey); | ||
| byte[] address = ecKey.getAddress(); | ||
| ByteString addressByte = ByteString.copyFrom(address); | ||
| chainManager.getAccountStore().put(addressByte.toByteArray(), | ||
| new AccountCapsule(Protocol.Account.newBuilder().setAddress(addressByte).build())); | ||
| WitnessCapsule witnessCapsule = new WitnessCapsule(addressByte); | ||
| chainManager.getWitnessScheduleStore().saveActiveWitnesses(new ArrayList<>()); | ||
| chainManager.addWitness(addressByte); | ||
| chainManager.getWitnessStore().put(address, witnessCapsule); | ||
| Block block = blockGenerate.getSignedBlock( | ||
| witnessCapsule.getAddress(), 1533529947843L, privateKey); | ||
| dbManager.pushBlock(new BlockCapsule(block)); | ||
|
|
||
| Map<ByteString, String> keys = addTestWitnessAndAccount(); | ||
| keys.put(addressByte, key); | ||
|
|
||
| // fund an owner; transfers go owner -> witness 'address' (an existing account) | ||
| ECKey ownerKey = new ECKey(Utils.getRandom()); | ||
| byte[] owner = ownerKey.getAddress(); | ||
| AccountCapsule ownerAccount = new AccountCapsule( | ||
| Protocol.Account.newBuilder().setAddress(ByteString.copyFrom(owner)).build()); | ||
| ownerAccount.setBalance(1_000_000_000L); | ||
| chainManager.getAccountStore().put(owner, ownerAccount); | ||
|
|
||
| long t = 1533529947843L; | ||
| long base = chainManager.getDynamicPropertiesStore().getLatestBlockHeaderNumber(); | ||
|
|
||
| // common ancestor P (empty) — fork point and tapos reference | ||
| BlockCapsule p = createTestBlockCapsule(t + 3000, base + 1, | ||
| chainManager.getDynamicPropertiesStore().getLatestBlockHeaderHash().getByteString(), keys); | ||
| dbManager.pushBlock(p); | ||
|
|
||
| long expiration = t + 1_000_000L; | ||
| BlockingQueue<FilterTriggerCapsule> queue = | ||
| ReflectUtils.getFieldValue(dbManager, "filterCapsuleQueue"); | ||
| queue.clear(); | ||
|
|
||
| // old branch: A carries a transfer; applied via the normal extend path | ||
| BlockCapsule a = blockWithTransfer(t + 6000, base + 2, p.getBlockId().getByteString(), keys, | ||
| transfer(owner, address, 1L, p, expiration)); | ||
| dbManager.pushBlock(a); | ||
| Assert.assertEquals("control: head should be A after normal extend", | ||
| a.getBlockId(), chainManager.getDynamicPropertiesStore().getLatestBlockHeaderHash()); | ||
| Assert.assertTrue("control: normal-path block A's logs must reach FULL stream (added)", | ||
| hasFullLogsFilter(queue, a, false)); | ||
|
|
||
| // heavier competing branch P -> B1 -> B2, each carrying a transfer, to force switchFork | ||
| BlockCapsule b1 = blockWithTransfer(t + 6001, base + 2, p.getBlockId().getByteString(), keys, | ||
| transfer(owner, address, 2L, p, expiration)); | ||
| dbManager.pushBlock(b1); // num <= head -> kept in khaosDb, no switch yet | ||
| BlockCapsule b2 = blockWithTransfer(t + 9000, base + 3, b1.getBlockId().getByteString(), keys, | ||
| transfer(owner, address, 3L, p, expiration)); | ||
| dbManager.pushBlock(b2); // num > head & parent != head -> triggers switchFork | ||
|
|
||
| Assert.assertEquals("reorg must switch the canonical head to the competing branch (B2)", | ||
| b2.getBlockId(), chainManager.getDynamicPropertiesStore().getLatestBlockHeaderHash()); | ||
|
|
||
| // reorg withdraws the orphaned old-branch logs (removed=true) | ||
| Assert.assertTrue("reorg: orphaned block A's logs must be withdrawn (removed=true)", | ||
| hasFullLogsFilter(queue, a, true)); | ||
| // the fix: new canonical blocks' logs are delivered (added, i.e. removed=false) | ||
| Assert.assertTrue("reorg: new canonical block B1's logs must reach FULL stream (added)", | ||
| hasFullLogsFilter(queue, b1, false)); | ||
| Assert.assertTrue("reorg: new canonical block B2's logs must reach FULL stream (added)", | ||
| hasFullLogsFilter(queue, b2, false)); | ||
| } | ||
|
|
||
| private TransactionCapsule transfer(byte[] owner, byte[] to, long amount, | ||
| BlockCapsule refBlock, long expiration) { | ||
| TransferContract contract = TransferContract.newBuilder() | ||
| .setOwnerAddress(ByteString.copyFrom(owner)) | ||
| .setToAddress(ByteString.copyFrom(to)) | ||
| .setAmount(amount).build(); | ||
| TransactionCapsule tx = new TransactionCapsule(contract, ContractType.TransferContract); | ||
| tx.setReference(refBlock.getNum(), refBlock.getBlockId().getBytes()); | ||
| tx.setExpiration(expiration); | ||
| return tx; | ||
| } | ||
|
|
||
| private BlockCapsule blockWithTransfer(long time, long number, ByteString parentHash, | ||
| Map<ByteString, String> keys, TransactionCapsule tx) { | ||
| ByteString witnessAddress = dposSlot.getScheduledWitness(dposSlot.getSlot(time)); | ||
| BlockCapsule blockCapsule = new BlockCapsule(number, Sha256Hash.wrap(parentHash), time, | ||
| witnessAddress); | ||
| blockCapsule.addTransaction(tx); | ||
| blockCapsule.generatedByMyself = true; | ||
| blockCapsule.setMerkleRoot(); | ||
| blockCapsule.sign(ByteArray.fromHexString(keys.get(witnessAddress))); | ||
| return blockCapsule; | ||
| } | ||
|
|
||
| private boolean hasFullLogsFilter(BlockingQueue<FilterTriggerCapsule> queue, BlockCapsule b, | ||
| boolean removed) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [NIT] Documentation & test backlog (2 items rolled up) Grouped as one comment since both are doc-or-test asks with no production-code impact.
Suggestion: (1) Add a |
||
| String blockHash = b.getBlockId().toString(); | ||
| return queue.stream() | ||
| .filter(c -> c instanceof LogsFilterCapsule) | ||
| .map(c -> (LogsFilterCapsule) c) | ||
| .anyMatch(c -> !c.isSolidified() && c.isRemoved() == removed | ||
| && blockHash.equals(c.getBlockHash())); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT] Document all known gaps between
reApplyLogsFilterandblockTriggerreApplyLogsFiltermirrors only the FULL JSON-RPC filter portion ofblockTrigger. Three additional responsibilities thatblockTriggercarries are absent on the reorg path, and none of them are recorded in the code:Switch-back (failed-rollback) path is uncovered.
reApplyLogsFilter(first)sits after the for-loop inswitchFork, so an exception thrown byapplyBlockpropagates before this line is reached — intentionally leaving the failed-rollback path without filter notifications. This is correct behaviour, but without a comment a future maintainer changing the exception handling (e.g. adding retry logic) may inadvertently introduce duplicate or missing filter events.Event-subscribe triggers are not fired. On the normal extend path
blockTriggeralso callspostBlockTrigger(block)andpostSolidityTrigger(newSolid)(event-subscribe v0), and updateslastUsedSolidityNum(event-plugin v1). None of these happen for blocks applied throughswitchFork. dApps relying on WebSocket event subscriptions will silently miss block and transaction events for every reorg'd block. This is documented in the PR description as a known limitation, but there is no in-code record.postSolidityFilteris not called here.blockTriggeralso callspostSolidityFilter(oldSolid, newSolid)for solidity-JSON-RPC-enabled nodes. Solidification events for reorg'd blocks do eventually arrive when the solidification height advances and a subsequentblockTriggercall invokespostSolidityFilteragainst the (now-updated) canonical chain — but this deferred-coverage contract is implicit and unrecorded.Future divergence risk. Because
reApplyLogsFilteris an explicit partial mirror ofblockTrigger, any future addition toblockTriggerwill silently be absent from the reorg path. Without a comment linking the two methods, this is easy to miss.Suggestion: Add a Javadoc or block comment above
reApplyLogsFilterlisting the out-of-scope items (event subscribe, postSolidityFilter deferred path, switch-back) and noting that this method must be kept in sync with the FULL-filter section ofblockTrigger. Add a short inline comment at thereApplyLogsFilter(first)call site noting that it only runs on the successful reorg path.