Skip to content

fix(framework): count pq_auth_sig in multisign fee and reverify checks #40

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.

Sign up for GitHub

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

Open
bladehan1 wants to merge 3 commits into
base: release_post_quantum
Choose a base branch
from
Open

fix(framework): count pq_auth_sig in multisign fee and reverify checks #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9dcb21f
fix(framework): count pq_auth_sig in multisign fee and reverify checks
bladehan1 Jun 23, 2026
68b9588
test(framework): simplify isSameSig pq tests and harden inactive-pq test
bladehan1 Jun 23, 2026
9094a1b
test(framework): make isSameSig visible for direct testing
bladehan1 Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions framework/src/main/java/org/tron/core/Wallet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,13 +676,21 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
}
}

List<ByteString> approveList = new ArrayList<>();
if (trx.getSignatureCount() > 0) {
List<ByteString> 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);
Expand Down
18 changes: 15 additions & 3 deletions framework/src/main/java/org/tron/core/db/Manager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -969,7 +970,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<Contract> contracts = trx.getInstance().getRawData().getContractList();
Expand Down Expand Up @@ -1223,12 +1224,14 @@ 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;
}

if (tx1.getInstance().getSignatureCount() != tx2.getInstance().getSignatureCount()) {
if (tx1.getInstance().getSignatureCount() != tx2.getInstance().getSignatureCount()
|| tx1.getInstance().getPqAuthSigCount() != tx2.getInstance().getPqAuthSigCount()) {
return false;
}

Expand All @@ -1242,6 +1245,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;
}

Expand Down
156 changes: 156 additions & 0 deletions framework/src/test/java/org/tron/core/WalletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1565,5 +1574,152 @@ 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 {
// 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);
Comment on lines +1682 to +1686

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Disable every PQ scheme for the inactive-gate test.

Line 1683 disables only FN_DSA_512, but Wallet gates on isAnyPqSchemeAllowed(). If another registered scheme such as ML_DSA_44 is enabled by default or a prior test, this no longer exercises the “no post-quantum scheme is activated” path.

Suggested fix 
   public void testApprovedListPqNotActivated() throws Exception {
     chainBaseManager.getDynamicPropertiesStore().saveAllowFnDsa512(0L);
+    chainBaseManager.getDynamicPropertiesStore().saveAllowMlDsa44(0L);
     FNDSA512 kp = new FNDSA512();
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/src/test/java/org/tron/core/WalletTest.java` around lines 1682 -
1683, The test method testApprovedListPqNotActivated() currently only disables
the FN_DSA_512 scheme via saveAllowFnDsa512(0L), but since Wallet gates on
isAnyPqSchemeAllowed(), other enabled schemes like ML_DSA_44 will cause the test
to not properly exercise the "no post-quantum scheme activated" path. Disable
all registered post-quantum schemes in this test by calling the appropriate save
methods on chainBaseManager.getDynamicPropertiesStore() for each PQ scheme (not
just FN_DSA_512) to ensure no post-quantum scheme is enabled before testing the
inactive gate scenario.

chainBaseManager.getDynamicPropertiesStore().saveAllowMlDsa44(0L);
Assert.assertFalse(chainBaseManager.getDynamicPropertiesStore().isAnyPqSchemeAllowed());
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());
}
}

8 changes: 4 additions & 4 deletions framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
Loading
Loading