From 9dcb21f886aa10e241da68559a5e5b270c1b95f1 Mon Sep 17 00:00:00 2001 From: bladehan1 Date: Tue, 23 Jun 2026 15:18:17 +0800 Subject: [PATCH 1/3] fix(framework): count pq_auth_sig in multisign fee and reverify checks PQ multisig support added pq_auth_sig but several signature-count checks kept summing ECDSA signatures only: - consumeMultiSignFee only checked getSignatureCount(), so PQ-only and hybrid ECDSA+PQ multisig transactions escaped the multisign fee. - isSameSig (used by getVerifyTxs to skip re-verifying transactions already verified in the pending pool) compared only the ECDSA signature list, so a block transaction with a substituted pq_auth_sig but the same txId (txId hashes raw_data only) could be wrongly marked verified and skip real signature validation. - Wallet.getTransactionApprovedList built the approved list from ECDSA signatures only, missing the pq_auth_sig branch already present in its sibling TransactionUtil.getTransactionSignWeight, so PQ signers never showed up in the approval status. Add unit tests covering PQ-only, hybrid, and regression cases for all three paths. Also wraps two pre-existing >100-char lines in ProposalUtilTest to keep checkstyleTest passing. --- .../src/main/java/org/tron/core/Wallet.java | 12 +- .../main/java/org/tron/core/db/Manager.java | 14 +- .../test/java/org/tron/core/WalletTest.java | 151 ++++++++++++ .../core/actuator/utils/ProposalUtilTest.java | 8 +- .../java/org/tron/core/db/ManagerTest.java | 217 ++++++++++++++++++ 5 files changed, 394 insertions(+), 8 deletions(-) diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 21297082b4..be0f49a8e9 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -676,13 +676,21 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) { } } + List approveList = new ArrayList<>(); if (trx.getSignatureCount() > 0) { - List approveList = new ArrayList<>(); byte[] hash = Sha256Hash.hash(CommonParameter .getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray()); TransactionCapsule.checkWeight(permission, trx.getSignatureList(), hash, approveList); - tswBuilder.addAllApprovedList(approveList); } + if (trx.getPqAuthSigCount() > 0) { + if (!chainBaseManager.getDynamicPropertiesStore().isAnyPqSchemeAllowed()) { + throw new PermissionException( + "pq_auth_sig not allowed: no post-quantum scheme is activated"); + } + TransactionCapsule.validatePQSignatureGetWeight(trx, permission, + chainBaseManager.getDynamicPropertiesStore(), approveList); + } + tswBuilder.addAllApprovedList(approveList); resultBuilder.setCode(TransactionApprovedList.Result.response_code.SUCCESS); } catch (SignatureFormatException signEx) { resultBuilder.setCode(TransactionApprovedList.Result.response_code.SIGNATURE_FORMAT_ERROR); diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java index 92231a762a..40552febfd 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -969,7 +969,7 @@ public boolean pushTransaction(final TransactionCapsule trx) public void consumeMultiSignFee(TransactionCapsule trx, TransactionTrace trace) throws AccountResourceInsufficientException { - if (trx.getInstance().getSignatureCount() > 1) { + if (trx.getInstance().getSignatureCount() + trx.getInstance().getPqAuthSigCount() > 1) { long fee = getDynamicPropertiesStore().getMultiSignFee(); boolean disableJavaLangMath = getDynamicPropertiesStore().disableJavaLangMath(); List contracts = trx.getInstance().getRawData().getContractList(); @@ -1228,7 +1228,8 @@ private boolean isSameSig(TransactionCapsule tx1, TransactionCapsule tx2) { return false; } - if (tx1.getInstance().getSignatureCount() != tx2.getInstance().getSignatureCount()) { + if (tx1.getInstance().getSignatureCount() != tx2.getInstance().getSignatureCount() + || tx1.getInstance().getPqAuthSigCount() != tx2.getInstance().getPqAuthSigCount()) { return false; } @@ -1242,6 +1243,15 @@ private boolean isSameSig(TransactionCapsule tx1, TransactionCapsule tx2) { } } + if (flag) { + for (int i = 0; i < tx1.getInstance().getPqAuthSigCount(); i++) { + if (!tx1.getInstance().getPqAuthSig(i).equals(tx2.getInstance().getPqAuthSig(i))) { + flag = false; + break; + } + } + } + return flag; } diff --git a/framework/src/test/java/org/tron/core/WalletTest.java b/framework/src/test/java/org/tron/core/WalletTest.java index f441ac856c..45e021ff93 100644 --- a/framework/src/test/java/org/tron/core/WalletTest.java +++ b/framework/src/test/java/org/tron/core/WalletTest.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import javax.annotation.Resource; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; @@ -51,8 +52,11 @@ import org.tron.common.BaseTest; import org.tron.common.TestConstants; import org.tron.common.crypto.ECKey; +import org.tron.common.crypto.pqc.FNDSA512; +import org.tron.common.crypto.pqc.PQSchemeRegistry; import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.ByteArray; +import org.tron.common.utils.Sha256Hash; import org.tron.common.utils.Utils; import org.tron.core.actuator.DelegateResourceActuator; import org.tron.core.actuator.FreezeBalanceActuator; @@ -88,6 +92,11 @@ import org.tron.protos.Protocol.BlockHeader; import org.tron.protos.Protocol.BlockHeader.raw; import org.tron.protos.Protocol.Exchange; +import org.tron.protos.Protocol.Key; +import org.tron.protos.Protocol.PQAuthSig; +import org.tron.protos.Protocol.PQScheme; +import org.tron.protos.Protocol.Permission; +import org.tron.protos.Protocol.Permission.PermissionType; import org.tron.protos.Protocol.Proposal; import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract; @@ -1565,5 +1574,147 @@ public void testApprovedListTooManySigs() { Assert.assertTrue(rejected.getResult().getMessage().contains("too many signatures")); assertEquals(0, rejected.getApprovedListCount()); } + + private static byte[] txIdOf(Transaction unsigned) { + return Sha256Hash.of(CommonParameter.getInstance().isECKeyCryptoEngine(), + unsigned.getRawData().toByteArray()).getBytes(); + } + + private Transaction buildApprovedListTransferTx(byte[] ownerAddress) { + return Transaction.newBuilder().setRawData( + Transaction.raw.newBuilder().addContract( + Contract.newBuilder().setType(ContractType.TransferContract) + .setParameter(Any.pack(TransferContract.newBuilder().setAmount(1) + .setOwnerAddress(ByteString.copyFrom(ownerAddress)) + .setToAddress(ByteString.copyFrom( + ByteArray.fromHexString(RECEIVER_ADDRESS))) + .build())).build()).build()).build(); + } + + @Test + public void testApprovedListPqOnlySigner() throws Exception { + chainBaseManager.getDynamicPropertiesStore().saveAllowFnDsa512(1L); + FNDSA512 kp = new FNDSA512(); + byte[] signerAddr = PQSchemeRegistry.computeAddress(PQScheme.FN_DSA_512, kp.getPublicKey()); + + ECKey accountKey = new ECKey(Utils.getRandom()); + byte[] ownerAddress = accountKey.getAddress(); + AccountCapsule owner = new AccountCapsule( + ByteString.copyFromUtf8("approved-pq-owner"), + ByteString.copyFrom(ownerAddress), + Protocol.AccountType.Normal, + initBalance); + Permission ownerPermission = Permission.newBuilder() + .setType(PermissionType.Owner) + .setPermissionName("owner") + .setThreshold(1) + .addKeys(Key.newBuilder().setAddress(ByteString.copyFrom(signerAddr)).setWeight(1L).build()) + .build(); + owner.updatePermissions(ownerPermission, null, Collections.emptyList()); + chainBaseManager.getAccountStore().put(ownerAddress, owner); + + Transaction unsigned = buildApprovedListTransferTx(ownerAddress); + byte[] sig = FNDSA512.sign(kp.getPrivateKey(), txIdOf(unsigned)); + Transaction signed = unsigned.toBuilder() + .addPqAuthSig(PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom(kp.getPublicKey())) + .setSignature(ByteString.copyFrom(sig)) + .build()) + .build(); + + GrpcAPI.TransactionApprovedList reply = wallet.getTransactionApprovedList(signed); + assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SUCCESS, + reply.getResult().getCode()); + assertEquals(1, reply.getApprovedListCount()); + assertEquals(ByteString.copyFrom(signerAddr), reply.getApprovedList(0)); + } + + @Test + public void testApprovedListHybridEcdsaAndPq() throws Exception { + chainBaseManager.getDynamicPropertiesStore().saveAllowFnDsa512(1L); + ECKey ecKey = new ECKey(Utils.getRandom()); + FNDSA512 kp = new FNDSA512(); + byte[] pqSignerAddr = PQSchemeRegistry.computeAddress(PQScheme.FN_DSA_512, kp.getPublicKey()); + + ECKey accountKey = new ECKey(Utils.getRandom()); + byte[] ownerAddress = accountKey.getAddress(); + AccountCapsule owner = new AccountCapsule( + ByteString.copyFromUtf8("approved-hybrid-owner"), + ByteString.copyFrom(ownerAddress), + Protocol.AccountType.Normal, + initBalance); + Permission ownerPermission = Permission.newBuilder() + .setType(PermissionType.Owner) + .setPermissionName("owner") + .setThreshold(2) + .addKeys(Key.newBuilder().setAddress(ByteString.copyFrom(ecKey.getAddress())) + .setWeight(1L).build()) + .addKeys(Key.newBuilder().setAddress(ByteString.copyFrom(pqSignerAddr)) + .setWeight(1L).build()) + .build(); + owner.updatePermissions(ownerPermission, null, Collections.emptyList()); + chainBaseManager.getAccountStore().put(ownerAddress, owner); + + Transaction unsigned = buildApprovedListTransferTx(ownerAddress); + + TransactionCapsule capsule = new TransactionCapsule(unsigned); + capsule.sign(ecKey.getPrivKeyBytes()); + ByteString ecdsaSig = capsule.getInstance().getSignature(0); + + byte[] pqSig = FNDSA512.sign(kp.getPrivateKey(), txIdOf(unsigned)); + Transaction signed = unsigned.toBuilder() + .addSignature(ecdsaSig) + .addPqAuthSig(PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom(kp.getPublicKey())) + .setSignature(ByteString.copyFrom(pqSig)) + .build()) + .build(); + + GrpcAPI.TransactionApprovedList reply = wallet.getTransactionApprovedList(signed); + assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SUCCESS, + reply.getResult().getCode()); + assertEquals(2, reply.getApprovedListCount()); + } + + @Test + public void testApprovedListPqNotActivated() throws Exception { + chainBaseManager.getDynamicPropertiesStore().saveAllowFnDsa512(0L); + FNDSA512 kp = new FNDSA512(); + byte[] signerAddr = PQSchemeRegistry.computeAddress(PQScheme.FN_DSA_512, kp.getPublicKey()); + + ECKey accountKey = new ECKey(Utils.getRandom()); + byte[] ownerAddress = accountKey.getAddress(); + AccountCapsule owner = new AccountCapsule( + ByteString.copyFromUtf8("approved-pq-inactive-owner"), + ByteString.copyFrom(ownerAddress), + Protocol.AccountType.Normal, + initBalance); + Permission ownerPermission = Permission.newBuilder() + .setType(PermissionType.Owner) + .setPermissionName("owner") + .setThreshold(1) + .addKeys(Key.newBuilder().setAddress(ByteString.copyFrom(signerAddr)).setWeight(1L).build()) + .build(); + owner.updatePermissions(ownerPermission, null, Collections.emptyList()); + chainBaseManager.getAccountStore().put(ownerAddress, owner); + + Transaction unsigned = buildApprovedListTransferTx(ownerAddress); + Transaction signed = unsigned.toBuilder() + .addPqAuthSig(PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom(kp.getPublicKey())) + .setSignature(ByteString.copyFrom(new byte[1])) + .build()) + .build(); + + GrpcAPI.TransactionApprovedList reply = wallet.getTransactionApprovedList(signed); + assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.OTHER_ERROR, + reply.getResult().getCode()); + Assert.assertTrue(reply.getResult().getMessage().contains( + "no post-quantum scheme is activated")); + assertEquals(0, reply.getApprovedListCount()); + } } diff --git a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java index cedbe691f7..2eef375c5b 100644 --- a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java +++ b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java @@ -816,8 +816,8 @@ public void validateAllowFnDsa512() { long maintenanceTimeInterval = forkUtils.getManager().getDynamicPropertiesStore() .getMaintenanceTimeInterval(); long hardForkTime = - ((ForkBlockVersionEnum.VERSION_4_8_2_PQ1.getHardForkTime() - 1) / maintenanceTimeInterval + 1) - * maintenanceTimeInterval; + ((ForkBlockVersionEnum.VERSION_4_8_2_PQ1.getHardForkTime() - 1) + / maintenanceTimeInterval + 1) * maintenanceTimeInterval; forkUtils.getManager().getDynamicPropertiesStore() .saveLatestBlockHeaderTimestamp(hardForkTime - 1); @@ -878,8 +878,8 @@ public void validateAllowMlDsa44() { long maintenanceTimeInterval = forkUtils.getManager().getDynamicPropertiesStore() .getMaintenanceTimeInterval(); long hardForkTime = - ((ForkBlockVersionEnum.VERSION_4_8_2_PQ1.getHardForkTime() - 1) / maintenanceTimeInterval + 1) - * maintenanceTimeInterval; + ((ForkBlockVersionEnum.VERSION_4_8_2_PQ1.getHardForkTime() - 1) + / maintenanceTimeInterval + 1) * maintenanceTimeInterval; forkUtils.getManager().getDynamicPropertiesStore() .saveLatestBlockHeaderTimestamp(hardForkTime - 1); diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index b31af7557f..09a3acda23 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -109,7 +109,10 @@ import org.tron.core.store.StoreFactory; import org.tron.protos.Protocol; import org.tron.protos.Protocol.Account; +import org.tron.protos.Protocol.AccountType; import org.tron.protos.Protocol.Block; +import org.tron.protos.Protocol.PQAuthSig; +import org.tron.protos.Protocol.PQScheme; import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.contract.AccountContract; @@ -210,6 +213,112 @@ public void updateRecentTransaction() throws Exception { Assert.assertEquals(trx.getTransactionId().toString(), item.getTransactionIds().get(0)); } + private static PQAuthSig dummyPqAuthSig() { + return PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom(new byte[1])) + .setSignature(ByteString.copyFrom(new byte[1])) + .build(); + } + + private AccountCapsule putMultiSignFeeOwner(String name) { + ECKey ownerKey = new ECKey(Utils.getRandom()); + byte[] ownerAddress = ownerKey.getAddress(); + AccountCapsule owner = new AccountCapsule(ByteString.copyFromUtf8(name), + ByteString.copyFrom(ownerAddress), AccountType.Normal, 10_000_000_000L); + chainManager.getAccountStore().put(ownerAddress, owner); + return owner; + } + + private TransactionCapsule buildTransferTxFrom(byte[] ownerAddress) { + TransferContract tc = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(ownerAddress)) + .setToAddress(ByteString.copyFrom(ownerAddress)) + .setAmount(1L) + .build(); + return new TransactionCapsule(tc, ContractType.TransferContract); + } + + @Test + public void consumeMultiSignFeePqOnlyCharged() throws Exception { + AccountCapsule owner = putMultiSignFeeOwner("multisignfee-pq"); + byte[] ownerAddress = owner.getAddress().toByteArray(); + + Transaction tx = buildTransferTxFrom(ownerAddress).getInstance().toBuilder() + .addPqAuthSig(dummyPqAuthSig()) + .addPqAuthSig(dummyPqAuthSig()) + .build(); + TransactionCapsule trx = new TransactionCapsule(tx); + TransactionTrace trace = new TransactionTrace(trx, StoreFactory.getInstance(), + new RuntimeImpl()); + + long fee = dbManager.getDynamicPropertiesStore().getMultiSignFee(); + dbManager.consumeMultiSignFee(trx, trace); + + Assert.assertEquals(10_000_000_000L - fee, + chainManager.getAccountStore().get(ownerAddress).getBalance()); + Assert.assertEquals(fee, trace.getReceipt().getMultiSignFee()); + } + + @Test + public void consumeMultiSignFeeHybridCharged() throws Exception { + AccountCapsule owner = putMultiSignFeeOwner("multisignfee-hybrid"); + byte[] ownerAddress = owner.getAddress().toByteArray(); + + Transaction tx = buildTransferTxFrom(ownerAddress).getInstance().toBuilder() + .addSignature(ByteString.copyFrom(new byte[]{1})) + .addPqAuthSig(dummyPqAuthSig()) + .build(); + TransactionCapsule trx = new TransactionCapsule(tx); + TransactionTrace trace = new TransactionTrace(trx, StoreFactory.getInstance(), + new RuntimeImpl()); + + long fee = dbManager.getDynamicPropertiesStore().getMultiSignFee(); + dbManager.consumeMultiSignFee(trx, trace); + + Assert.assertEquals(10_000_000_000L - fee, + chainManager.getAccountStore().get(ownerAddress).getBalance()); + Assert.assertEquals(fee, trace.getReceipt().getMultiSignFee()); + } + + @Test + public void consumeMultiSignFeeSingleEcdsaNotCharged() throws Exception { + AccountCapsule owner = putMultiSignFeeOwner("multisignfee-single-ecdsa"); + byte[] ownerAddress = owner.getAddress().toByteArray(); + + Transaction tx = buildTransferTxFrom(ownerAddress).getInstance().toBuilder() + .addSignature(ByteString.copyFrom(new byte[]{1})) + .build(); + TransactionCapsule trx = new TransactionCapsule(tx); + TransactionTrace trace = new TransactionTrace(trx, StoreFactory.getInstance(), + new RuntimeImpl()); + + dbManager.consumeMultiSignFee(trx, trace); + + Assert.assertEquals(10_000_000_000L, + chainManager.getAccountStore().get(ownerAddress).getBalance()); + Assert.assertEquals(0L, trace.getReceipt().getMultiSignFee()); + } + + @Test + public void consumeMultiSignFeeSinglePqNotCharged() throws Exception { + AccountCapsule owner = putMultiSignFeeOwner("multisignfee-single-pq"); + byte[] ownerAddress = owner.getAddress().toByteArray(); + + Transaction tx = buildTransferTxFrom(ownerAddress).getInstance().toBuilder() + .addPqAuthSig(dummyPqAuthSig()) + .build(); + TransactionCapsule trx = new TransactionCapsule(tx); + TransactionTrace trace = new TransactionTrace(trx, StoreFactory.getInstance(), + new RuntimeImpl()); + + dbManager.consumeMultiSignFee(trx, trace); + + Assert.assertEquals(10_000_000_000L, + chainManager.getAccountStore().get(ownerAddress).getBalance()); + Assert.assertEquals(0L, trace.getReceipt().getMultiSignFee()); + } + @Test public void setBlockReference() throws ContractExeException, UnLinkedBlockException, ValidateScheduleException, @@ -885,6 +994,114 @@ public void getVerifyTxsTest() { Assert.assertEquals(txs.size(), 1); } + @Test + public void getVerifyTxsPqAuthSigContentDifferRequiresReverify() { + // Same txId (raw_data is identical), different pq_auth_sig content: isSameSig must + // not treat these as the same signature, or the block tx would skip real verification. + TransferContract c1 = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) + .setAmount(1).build(); + TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); + + PQAuthSig sigA = PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom("pubA".getBytes())) + .setSignature(ByteString.copyFrom("sigA".getBytes())) + .build(); + PQAuthSig sigB = PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom("pubB".getBytes())) + .setSignature(ByteString.copyFrom("sigB".getBytes())) + .build(); + + Transaction pendingTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); + Transaction blockTx = t1.getInstance().toBuilder().addPqAuthSig(sigB).build(); + Assert.assertEquals(new TransactionCapsule(pendingTx).getTransactionId(), + new TransactionCapsule(blockTx).getTransactionId()); + + dbManager.getPendingTransactions().clear(); + try { + dbManager.getPendingTransactions().add(new TransactionCapsule(pendingTx)); + + List list = new ArrayList<>(); + list.add(blockTx); + BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); + + List txs = dbManager.getVerifyTxs(capsule); + Assert.assertEquals(1, txs.size()); + } finally { + dbManager.getPendingTransactions().clear(); + } + } + + @Test + public void getVerifyTxsPqAuthSigCountDifferRequiresReverify() { + // Same txId, pending has no pq_auth_sig but the block tx adds one: count mismatch + // must force re-verification rather than matching on the (empty) ECDSA list alone. + TransferContract c1 = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) + .setAmount(1).build(); + TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); + + Transaction pendingTx = t1.getInstance(); + Transaction blockTx = t1.getInstance().toBuilder() + .addPqAuthSig(PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom("pubA".getBytes())) + .setSignature(ByteString.copyFrom("sigA".getBytes())) + .build()) + .build(); + Assert.assertEquals(new TransactionCapsule(pendingTx).getTransactionId(), + new TransactionCapsule(blockTx).getTransactionId()); + + dbManager.getPendingTransactions().clear(); + try { + dbManager.getPendingTransactions().add(new TransactionCapsule(pendingTx)); + + List list = new ArrayList<>(); + list.add(blockTx); + BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); + + List txs = dbManager.getVerifyTxs(capsule); + Assert.assertEquals(1, txs.size()); + } finally { + dbManager.getPendingTransactions().clear(); + } + } + + @Test + public void getVerifyTxsPqAuthSigIdenticalSkipsReverify() { + // Regression: identical pq_auth_sig content on both sides must still take the + // already-verified fast path, so the fix does not regress the optimization. + TransferContract c1 = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) + .setAmount(1).build(); + TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); + + PQAuthSig sigA = PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom("pubA".getBytes())) + .setSignature(ByteString.copyFrom("sigA".getBytes())) + .build(); + + Transaction pendingTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); + Transaction blockTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); + + dbManager.getPendingTransactions().clear(); + try { + dbManager.getPendingTransactions().add(new TransactionCapsule(pendingTx)); + + List list = new ArrayList<>(); + list.add(blockTx); + BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); + + List txs = dbManager.getVerifyTxs(capsule); + Assert.assertEquals(0, txs.size()); + } finally { + dbManager.getPendingTransactions().clear(); + } + } + @Test public void getVerifyTxsSkipsBlockWhenPermissionTxAlreadyConsumed() throws Exception { // Scenario: a permission-change tx (A) for owner X has been processed and consumed, From 68b958837bc68fe6e0394040f59522a219b324a9 Mon Sep 17 00:00:00 2001 From: bladehan1 Date: Tue, 23 Jun 2026 16:11:25 +0800 Subject: [PATCH 2/3] test(framework): simplify isSameSig pq tests and harden inactive-pq test Collapse the three getVerifyTxs pq_auth_sig tests onto one shared helper instead of repeating the same transaction/pending-pool/ try-finally scaffolding in each. testApprovedListPqNotActivated only disabled FN_DSA_512, but Wallet gates on isAnyPqSchemeAllowed() across every registered scheme. Disable ML_DSA_44 too and assert isAnyPqSchemeAllowed() is false before building the transaction, so the test still exercises the intended path if another scheme is enabled by default or by a prior test. --- .../test/java/org/tron/core/WalletTest.java | 5 + .../java/org/tron/core/db/ManagerTest.java | 115 ++++++------------ 2 files changed, 45 insertions(+), 75 deletions(-) diff --git a/framework/src/test/java/org/tron/core/WalletTest.java b/framework/src/test/java/org/tron/core/WalletTest.java index 45e021ff93..2512d4f104 100644 --- a/framework/src/test/java/org/tron/core/WalletTest.java +++ b/framework/src/test/java/org/tron/core/WalletTest.java @@ -1680,7 +1680,12 @@ public void testApprovedListHybridEcdsaAndPq() throws Exception { @Test public void testApprovedListPqNotActivated() throws Exception { + // Disable every registered PQ scheme, not just FN_DSA_512: Wallet gates on + // isAnyPqSchemeAllowed(), so a scheme left enabled by default or a prior test + // would silently skip the "no post-quantum scheme is activated" path. chainBaseManager.getDynamicPropertiesStore().saveAllowFnDsa512(0L); + chainBaseManager.getDynamicPropertiesStore().saveAllowMlDsa44(0L); + Assert.assertFalse(chainBaseManager.getDynamicPropertiesStore().isAnyPqSchemeAllowed()); FNDSA512 kp = new FNDSA512(); byte[] signerAddr = PQSchemeRegistry.computeAddress(PQScheme.FN_DSA_512, kp.getPublicKey()); diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index 09a3acda23..b3921d7c3e 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -24,6 +24,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -994,28 +995,29 @@ public void getVerifyTxsTest() { Assert.assertEquals(txs.size(), 1); } - @Test - public void getVerifyTxsPqAuthSigContentDifferRequiresReverify() { - // Same txId (raw_data is identical), different pq_auth_sig content: isSameSig must - // not treat these as the same signature, or the block tx would skip real verification. + private static PQAuthSig pqAuthSig(String publicKey, String signature) { + return PQAuthSig.newBuilder() + .setScheme(PQScheme.FN_DSA_512) + .setPublicKey(ByteString.copyFrom(publicKey.getBytes())) + .setSignature(ByteString.copyFrom(signature.getBytes())) + .build(); + } + + /** + * Builds two transactions sharing the same raw_data (hence the same txId) but with + * {@code pendingSigs}/{@code blockSigs} as their respective pq_auth_sig lists, puts the + * first in the pending pool, and asserts how many of the second {@code getVerifyTxs} + * sends back for re-verification. + */ + private void assertGetVerifyTxsPqAuthSig(List pendingSigs, List blockSigs, + int expectedReverifyCount) { TransferContract c1 = TransferContract.newBuilder() .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) .setAmount(1).build(); - TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); - - PQAuthSig sigA = PQAuthSig.newBuilder() - .setScheme(PQScheme.FN_DSA_512) - .setPublicKey(ByteString.copyFrom("pubA".getBytes())) - .setSignature(ByteString.copyFrom("sigA".getBytes())) - .build(); - PQAuthSig sigB = PQAuthSig.newBuilder() - .setScheme(PQScheme.FN_DSA_512) - .setPublicKey(ByteString.copyFrom("pubB".getBytes())) - .setSignature(ByteString.copyFrom("sigB".getBytes())) - .build(); + Transaction base = new TransactionCapsule(c1, ContractType.TransferContract).getInstance(); - Transaction pendingTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); - Transaction blockTx = t1.getInstance().toBuilder().addPqAuthSig(sigB).build(); + Transaction pendingTx = base.toBuilder().addAllPqAuthSig(pendingSigs).build(); + Transaction blockTx = base.toBuilder().addAllPqAuthSig(blockSigs).build(); Assert.assertEquals(new TransactionCapsule(pendingTx).getTransactionId(), new TransactionCapsule(blockTx).getTransactionId()); @@ -1028,78 +1030,41 @@ public void getVerifyTxsPqAuthSigContentDifferRequiresReverify() { BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); List txs = dbManager.getVerifyTxs(capsule); - Assert.assertEquals(1, txs.size()); + Assert.assertEquals(expectedReverifyCount, txs.size()); } finally { dbManager.getPendingTransactions().clear(); } } + @Test + public void getVerifyTxsPqAuthSigContentDifferRequiresReverify() { + // Same txId, different pq_auth_sig content: isSameSig must not treat these as the + // same signature, or the block tx would skip real verification. + assertGetVerifyTxsPqAuthSig( + Collections.singletonList(pqAuthSig("pubA", "sigA")), + Collections.singletonList(pqAuthSig("pubB", "sigB")), + 1); + } + @Test public void getVerifyTxsPqAuthSigCountDifferRequiresReverify() { // Same txId, pending has no pq_auth_sig but the block tx adds one: count mismatch // must force re-verification rather than matching on the (empty) ECDSA list alone. - TransferContract c1 = TransferContract.newBuilder() - .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) - .setAmount(1).build(); - TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); - - Transaction pendingTx = t1.getInstance(); - Transaction blockTx = t1.getInstance().toBuilder() - .addPqAuthSig(PQAuthSig.newBuilder() - .setScheme(PQScheme.FN_DSA_512) - .setPublicKey(ByteString.copyFrom("pubA".getBytes())) - .setSignature(ByteString.copyFrom("sigA".getBytes())) - .build()) - .build(); - Assert.assertEquals(new TransactionCapsule(pendingTx).getTransactionId(), - new TransactionCapsule(blockTx).getTransactionId()); - - dbManager.getPendingTransactions().clear(); - try { - dbManager.getPendingTransactions().add(new TransactionCapsule(pendingTx)); - - List list = new ArrayList<>(); - list.add(blockTx); - BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); - - List txs = dbManager.getVerifyTxs(capsule); - Assert.assertEquals(1, txs.size()); - } finally { - dbManager.getPendingTransactions().clear(); - } + assertGetVerifyTxsPqAuthSig( + Collections.emptyList(), + Collections.singletonList(pqAuthSig("pubA", "sigA")), + 1); } @Test public void getVerifyTxsPqAuthSigIdenticalSkipsReverify() { // Regression: identical pq_auth_sig content on both sides must still take the // already-verified fast path, so the fix does not regress the optimization. - TransferContract c1 = TransferContract.newBuilder() - .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) - .setAmount(1).build(); - TransactionCapsule t1 = new TransactionCapsule(c1, ContractType.TransferContract); - - PQAuthSig sigA = PQAuthSig.newBuilder() - .setScheme(PQScheme.FN_DSA_512) - .setPublicKey(ByteString.copyFrom("pubA".getBytes())) - .setSignature(ByteString.copyFrom("sigA".getBytes())) - .build(); - - Transaction pendingTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); - Transaction blockTx = t1.getInstance().toBuilder().addPqAuthSig(sigA).build(); - - dbManager.getPendingTransactions().clear(); - try { - dbManager.getPendingTransactions().add(new TransactionCapsule(pendingTx)); - - List list = new ArrayList<>(); - list.add(blockTx); - BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); - - List txs = dbManager.getVerifyTxs(capsule); - Assert.assertEquals(0, txs.size()); - } finally { - dbManager.getPendingTransactions().clear(); - } + PQAuthSig sigA = pqAuthSig("pubA", "sigA"); + assertGetVerifyTxsPqAuthSig( + Collections.singletonList(sigA), + Collections.singletonList(sigA), + 0); } @Test From 9094a1b556371352d0c7b323edf7629cfd47836c Mon Sep 17 00:00:00 2001 From: bladehan1 Date: Tue, 23 Jun 2026 16:35:40 +0800 Subject: [PATCH 3/3] test(framework): make isSameSig visible for direct testing Mark isSameSig package-private with @VisibleForTesting (precedented in this codebase by TransactionCapsule.logSlowSigVerify and several other @VisibleForTesting members) instead of exercising it only indirectly through getVerifyTxs's pending-pool/BlockCapsule scaffolding. Add 5 direct isSameSig tests covering null, pq_auth_sig content/count mismatch, identical pq_auth_sig, and the exact bug scenario (matching ECDSA list masking a differing pq_auth_sig list). Keep one getVerifyTxs-level integration test (getVerifyTxsClosesPqAuthSigBypass) as proof the fix is actually wired into the real bypass path, since a unit test of isSameSig alone can't prove that. --- .../main/java/org/tron/core/db/Manager.java | 4 +- .../java/org/tron/core/db/ManagerTest.java | 108 +++++++++++------- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java index 40552febfd..8e67b62dc1 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -9,6 +9,7 @@ import static org.tron.protos.Protocol.Transaction.Contract.ContractType.TransferContract; import static org.tron.protos.Protocol.Transaction.Result.contractResult.SUCCESS; +import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.collect.Lists; @@ -1223,7 +1224,8 @@ private void switchFork(BlockCapsule newHead) } - private boolean isSameSig(TransactionCapsule tx1, TransactionCapsule tx2) { + @VisibleForTesting + boolean isSameSig(TransactionCapsule tx1, TransactionCapsule tx2) { if (tx1 == null || tx2 == null) { return false; } diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index b3921d7c3e..3dc5bab277 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -24,7 +24,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -1003,21 +1002,73 @@ private static PQAuthSig pqAuthSig(String publicKey, String signature) { .build(); } - /** - * Builds two transactions sharing the same raw_data (hence the same txId) but with - * {@code pendingSigs}/{@code blockSigs} as their respective pq_auth_sig lists, puts the - * first in the pending pool, and asserts how many of the second {@code getVerifyTxs} - * sends back for re-verification. - */ - private void assertGetVerifyTxsPqAuthSig(List pendingSigs, List blockSigs, - int expectedReverifyCount) { + private TransactionCapsule pqAndEcdsaSignedCapsule(String ecdsaSig, PQAuthSig... pqSigs) { + TransferContract c = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) + .setAmount(1).build(); + Transaction.Builder builder = new TransactionCapsule(c, ContractType.TransferContract) + .getInstance().toBuilder(); + if (ecdsaSig != null) { + builder.addSignature(ByteString.copyFrom(ecdsaSig.getBytes())); + } + for (PQAuthSig sig : pqSigs) { + builder.addPqAuthSig(sig); + } + return new TransactionCapsule(builder.build()); + } + + // isSameSig is package-private (@VisibleForTesting) so these exercise it directly, + // instead of indirectly through getVerifyTxs's pending-pool/BlockCapsule scaffolding. + + @Test + public void isSameSigNullIsFalse() { + TransactionCapsule tx = pqAndEcdsaSignedCapsule(null); + Assert.assertFalse(dbManager.isSameSig(null, tx)); + Assert.assertFalse(dbManager.isSameSig(tx, null)); + } + + @Test + public void isSameSigPqAuthSigContentDiffers() { + Assert.assertFalse(dbManager.isSameSig( + pqAndEcdsaSignedCapsule(null, pqAuthSig("pubA", "sigA")), + pqAndEcdsaSignedCapsule(null, pqAuthSig("pubB", "sigB")))); + } + + @Test + public void isSameSigPqAuthSigCountDiffers() { + Assert.assertFalse(dbManager.isSameSig( + pqAndEcdsaSignedCapsule(null), + pqAndEcdsaSignedCapsule(null, pqAuthSig("pubA", "sigA")))); + } + + @Test + public void isSameSigPqAuthSigIdenticalIsTrue() { + PQAuthSig sig = pqAuthSig("pubA", "sigA"); + Assert.assertTrue(dbManager.isSameSig( + pqAndEcdsaSignedCapsule(null, sig), + pqAndEcdsaSignedCapsule(null, sig))); + } + + @Test + public void isSameSigHybridEcdsaMatchesButPqDiffers() { + // The exact bug scenario: an identical ECDSA list must not mask a differing + // pq_auth_sig list. + Assert.assertFalse(dbManager.isSameSig( + pqAndEcdsaSignedCapsule("a", pqAuthSig("pubA", "sigA")), + pqAndEcdsaSignedCapsule("a", pqAuthSig("pubB", "sigB")))); + } + + @Test + public void getVerifyTxsClosesPqAuthSigBypass() { + // Integration proof: a block tx sharing the pending tx's txId (raw_data is + // identical) but carrying a different pq_auth_sig must still be sent back for + // re-verification by getVerifyTxs, not silently marked already-verified. TransferContract c1 = TransferContract.newBuilder() .setOwnerAddress(ByteString.copyFrom("f1".getBytes())) .setAmount(1).build(); Transaction base = new TransactionCapsule(c1, ContractType.TransferContract).getInstance(); - - Transaction pendingTx = base.toBuilder().addAllPqAuthSig(pendingSigs).build(); - Transaction blockTx = base.toBuilder().addAllPqAuthSig(blockSigs).build(); + Transaction pendingTx = base.toBuilder().addPqAuthSig(pqAuthSig("pubA", "sigA")).build(); + Transaction blockTx = base.toBuilder().addPqAuthSig(pqAuthSig("pubB", "sigB")).build(); Assert.assertEquals(new TransactionCapsule(pendingTx).getTransactionId(), new TransactionCapsule(blockTx).getTransactionId()); @@ -1030,43 +1081,12 @@ private void assertGetVerifyTxsPqAuthSig(List pendingSigs, List txs = dbManager.getVerifyTxs(capsule); - Assert.assertEquals(expectedReverifyCount, txs.size()); + Assert.assertEquals(1, txs.size()); } finally { dbManager.getPendingTransactions().clear(); } } - @Test - public void getVerifyTxsPqAuthSigContentDifferRequiresReverify() { - // Same txId, different pq_auth_sig content: isSameSig must not treat these as the - // same signature, or the block tx would skip real verification. - assertGetVerifyTxsPqAuthSig( - Collections.singletonList(pqAuthSig("pubA", "sigA")), - Collections.singletonList(pqAuthSig("pubB", "sigB")), - 1); - } - - @Test - public void getVerifyTxsPqAuthSigCountDifferRequiresReverify() { - // Same txId, pending has no pq_auth_sig but the block tx adds one: count mismatch - // must force re-verification rather than matching on the (empty) ECDSA list alone. - assertGetVerifyTxsPqAuthSig( - Collections.emptyList(), - Collections.singletonList(pqAuthSig("pubA", "sigA")), - 1); - } - - @Test - public void getVerifyTxsPqAuthSigIdenticalSkipsReverify() { - // Regression: identical pq_auth_sig content on both sides must still take the - // already-verified fast path, so the fix does not regress the optimization. - PQAuthSig sigA = pqAuthSig("pubA", "sigA"); - assertGetVerifyTxsPqAuthSig( - Collections.singletonList(sigA), - Collections.singletonList(sigA), - 0); - } - @Test public void getVerifyTxsSkipsBlockWhenPermissionTxAlreadyConsumed() throws Exception { // Scenario: a permission-change tx (A) for owner X has been processed and consumed,