refactor(pqc): harden ALLOW_FN_DSA_512 proposal validation and PQ-only witness init

Author: Federico2014

actuator/src/main/java/org/tron/core/utils/ProposalUtil.java

@@ -942,13 +942,18 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore,
         break;
       }
       case ALLOW_FN_DSA_512: {
-        if (dynamicPropertiesStore.getAllowFnDsa512() == 1) {
+        if (!forkController.pass(ForkBlockVersionEnum.VERSION_4_8_2)) {
           throw new ContractValidateException(
-              "[ALLOW_FN_DSA_512] has been valid, no need to propose again");
+              "Bad chain parameter id [ALLOW_FN_DSA_512]");
         }
-        if (value != 1) {
+        if (value != 0 && value != 1) {
           throw new ContractValidateException(
-              "This value[ALLOW_FN_DSA_512] is only allowed to be 1");
+              "This value[ALLOW_FN_DSA_512] is only allowed to be 0 or 1");
+        }
+        if (dynamicPropertiesStore.getAllowFnDsa512() == value) {
+          throw new ContractValidateException(
+              "[ALLOW_FN_DSA_512] has been set to " + value
+                  + ", no need to propose again");
         }
         break;
       }
@@ -1041,7 +1046,7 @@ public enum ProposalType {         // current value, value range
     ALLOW_TVM_OSAKA(96), // 0, 1
     ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1
     ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1
-    ALLOW_FN_DSA_512(100); // 0, 1
+    ALLOW_FN_DSA_512(99); // 0, 1
 
     private long code;
 

framework/src/main/java/org/tron/core/config/args/Args.java

@@ -962,29 +962,26 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) {
     if (config.hasPath(ConfigKey.LOCAL_WITNESS_PQ_KEYS)) {
       List<String> pqEntries = config.getStringList(ConfigKey.LOCAL_WITNESS_PQ_KEYS);
       if (!pqEntries.isEmpty()) {
-        localWitnesses = new LocalWitnesses();
-        // Scheme must be applied before keypairs — key-length validation depends on it.
+        PQScheme scheme = PQScheme.FN_DSA_512;
         if (config.hasPath(ConfigKey.LOCAL_WITNESS_PQ_SCHEME)) {
           String schemeName = config.getString(ConfigKey.LOCAL_WITNESS_PQ_SCHEME);
           try {
-            PQScheme scheme = PQScheme.valueOf(schemeName);
-            if (!WITNESS_PQ_SCHEMES.contains(scheme)) {
-              throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_PQ_SCHEME
-                  + ": " + schemeName + "; valid values: " + WITNESS_PQ_SCHEMES,
-                  TronError.ErrCode.WITNESS_INIT);
-            }
-            localWitnesses.setPqScheme(scheme);
+            scheme = PQScheme.valueOf(schemeName);
           } catch (IllegalArgumentException e) {
             throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_PQ_SCHEME
                 + ": " + schemeName, TronError.ErrCode.WITNESS_INIT);
           }
+          if (!WITNESS_PQ_SCHEMES.contains(scheme)) {
+            throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_PQ_SCHEME
+                + ": " + schemeName + "; valid values: " + WITNESS_PQ_SCHEMES,
+                TronError.ErrCode.WITNESS_INIT);
+          }
         }
         // Each entry is the extended private key f‖g‖F‖h (priv ‖ pub) hex,
         // sized (privLen + pubLen) bytes for the active scheme. We split here
         // so downstream consumers (ConsensusService, LocalWitnesses) keep the
         // same priv/pub split they already use — derivePublicKey(priv) replaces
         // the previous explicit `pub` config field.
-        PQScheme scheme = localWitnesses.getPqScheme();
         int privHexLen = PQSchemeRegistry.getPrivateKeyLength(scheme) * 2;
         int extHexLen = privHexLen + PQSchemeRegistry.getPublicKeyLength(scheme) * 2;
         List<String> pqPrivateKeys = new ArrayList<>(pqEntries.size());
@@ -1002,11 +999,8 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) {
           pqPrivateKeys.add(stripped.substring(0, privHexLen));
           pqPublicKeys.add(stripped.substring(privHexLen));
         }
-        localWitnesses.setPqKeypairs(pqPrivateKeys, pqPublicKeys);
-        byte[] address = WitnessInitializer.resolvePqAuthSigAddress(lwConfig.getAccountAddress());
-        if (address != null) {
-          localWitnesses.setWitnessAccountAddress(address);
-        }
+        localWitnesses = WitnessInitializer.initFromPQOnly(
+            scheme, pqPrivateKeys, pqPublicKeys, lwConfig.getAccountAddress());
         return;
       }
     }

framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java

@@ -7,13 +7,15 @@
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.tron.common.crypto.SignInterface;
+import org.tron.common.crypto.pqc.PQSchemeRegistry;
 import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.Commons;
 import org.tron.common.utils.LocalWitnesses;
 import org.tron.core.exception.CipherException;
 import org.tron.core.exception.TronError;
 import org.tron.keystore.Credentials;
 import org.tron.keystore.WalletUtils;
+import org.tron.protos.Protocol.PQScheme;
 
 @Slf4j
 public class WitnessInitializer {
@@ -113,42 +115,39 @@ public static LocalWitnesses initFromKeystore(
   }
 
   /**
-   * Init for PQ-only witness nodes (no legacy ECDSA key). The witness account
-   * address must be supplied explicitly because there is no ECDSA key to derive it from.
+   * Init for PQ-only witness nodes (no legacy ECDSA key). When
+   * {@code witnessAccountAddress} is blank, the address is derived from the
+   * first PQ public key via {@link PQSchemeRegistry#computeAddress(PQScheme,
+   * byte[])}.
    */
-  public static LocalWitnesses initFromPQOnly(String witnessAccountAddress) {
-    if (StringUtils.isBlank(witnessAccountAddress)) {
-      throw new TronError(
-          "localWitnessAccountAddress must be set for PQ-only witness nodes",
-          TronError.ErrCode.WITNESS_INIT);
-    }
-    byte[] address = Commons.decodeFromBase58Check(witnessAccountAddress);
-    if (address == null) {
+  public static LocalWitnesses initFromPQOnly(PQScheme scheme,
+      List<String> pqPrivateKeys, List<String> pqPublicKeys,
+      String witnessAccountAddress) {
+    if (pqPublicKeys == null || pqPublicKeys.isEmpty()) {
       throw new TronError(
-          "LocalWitnessAccountAddress format is incorrect",
+          "PQ public keys must be set for PQ-only witness nodes",
           TronError.ErrCode.WITNESS_INIT);
     }
     LocalWitnesses witnesses = new LocalWitnesses();
-    witnesses.initWitnessAccountAddress(address, false);
-    logger.debug("Initialised PQ-only witness with address {}", witnessAccountAddress);
-    return witnesses;
-  }
+    witnesses.setPqScheme(scheme);
+    witnesses.setPqKeypairs(pqPrivateKeys, pqPublicKeys);
 
-  /**
-   * Resolve witness address for PQ seed configuration.
-   */
-  public static byte[] resolvePqAuthSigAddress(String witnessAccountAddress) {
-    if (StringUtils.isEmpty(witnessAccountAddress)) {
-      return null;
-    }
-    byte[] address = Commons.decodeFromBase58Check(witnessAccountAddress);
-    if (address != null) {
-      logger.debug("Got localWitnessAccountAddress from config.conf");
+    byte[] address;
+    if (StringUtils.isBlank(witnessAccountAddress)) {
+      byte[] firstPubKey = ByteArray.fromHexString(pqPublicKeys.get(0));
+      address = PQSchemeRegistry.computeAddress(scheme, firstPubKey);
+      logger.debug("Derived PQ-only witness address from public key");
     } else {
-      throw new TronError("LocalWitnessAccountAddress format from config is incorrect",
-          TronError.ErrCode.WITNESS_INIT);
+      address = Commons.decodeFromBase58Check(witnessAccountAddress);
+      if (address == null) {
+        throw new TronError(
+            "LocalWitnessAccountAddress format is incorrect",
+            TronError.ErrCode.WITNESS_INIT);
+      }
+      logger.debug("Got localWitnessAccountAddress from config.conf");
     }
-    return address;
+    witnesses.initWitnessAccountAddress(address, false);
+    return witnesses;
   }
 
   static byte[] resolveWitnessAddress(

framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java

@@ -801,26 +801,62 @@ public void blockVersionCheck() {
   @Test
   public void validateAllowFnDsa512() {
     long code = ProposalType.ALLOW_FN_DSA_512.getCode();
+    ThrowingRunnable proposeZero = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils,
+        code, 0);
+    ThrowingRunnable proposeOne = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils,
+        code, 1);
+    ThrowingRunnable proposeTwo = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils,
+        code, 2);
 
-    ContractValidateException thrown = assertThrows(ContractValidateException.class,
-        () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 0));
-    assertEquals("This value[ALLOW_FN_DSA_512] is only allowed to be 1", thrown.getMessage());
+    byte[] stats = new byte[27];
+    forkUtils.getManager().getDynamicPropertiesStore()
+        .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_1.getValue(), stats);
+    long maintenanceTimeInterval = forkUtils.getManager().getDynamicPropertiesStore()
+        .getMaintenanceTimeInterval();
+    long hardForkTime =
+        ((ForkBlockVersionEnum.VERSION_4_8_2.getHardForkTime() - 1) / maintenanceTimeInterval + 1)
+            * maintenanceTimeInterval;
+    forkUtils.getManager().getDynamicPropertiesStore()
+        .saveLatestBlockHeaderTimestamp(hardForkTime - 1);
 
-    thrown = assertThrows(ContractValidateException.class,
-        () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 2));
-    assertEquals("This value[ALLOW_FN_DSA_512] is only allowed to be 1", thrown.getMessage());
+    // 1) before fork 4.8.2 -> rejected
+    ContractValidateException thrown = assertThrows(ContractValidateException.class, proposeOne);
+    assertEquals("Bad chain parameter id [ALLOW_FN_DSA_512]", thrown.getMessage());
 
+    forkUtils.getManager().getDynamicPropertiesStore()
+        .saveLatestBlockHeaderTimestamp(hardForkTime + 1);
+    Arrays.fill(stats, (byte) 1);
+    forkUtils.getManager().getDynamicPropertiesStore()
+        .statsByVersion(ForkBlockVersionEnum.VERSION_4_8_2.getValue(), stats);
+
+    // 2) value not in {0, 1} -> rejected
+    thrown = assertThrows(ContractValidateException.class, proposeTwo);
+    assertEquals("This value[ALLOW_FN_DSA_512] is only allowed to be 0 or 1", thrown.getMessage());
+
+    // 3) current value is 0 (default), proposing 0 again -> rejected
+    thrown = assertThrows(ContractValidateException.class, proposeZero);
+    assertEquals("[ALLOW_FN_DSA_512] has been set to 0, no need to propose again",
+        thrown.getMessage());
+
+    // 4) value=1 to enable -> ok
     try {
-      ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 1);
-    } catch (ContractValidateException e) {
-      Assert.fail("value=1 should be accepted: " + e.getMessage());
+      proposeOne.run();
+    } catch (Throwable e) {
+      Assert.fail("Should pass when toggling 0 -> 1: " + e.getMessage());
     }
 
+    // 5) after activation, proposing 1 again -> rejected
     dynamicPropertiesStore.saveAllowFnDsa512(1L);
-    thrown = assertThrows(ContractValidateException.class,
-        () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, code, 1));
-    assertEquals("[ALLOW_FN_DSA_512] has been valid, no need to propose again",
+    thrown = assertThrows(ContractValidateException.class, proposeOne);
+    assertEquals("[ALLOW_FN_DSA_512] has been set to 1, no need to propose again",
         thrown.getMessage());
+
+    // 6) value=0 to disable -> ok (toggle back off)
+    try {
+      proposeZero.run();
+    } catch (Throwable e) {
+      Assert.fail("Should pass when toggling 1 -> 0: " + e.getMessage());
+    }
     dynamicPropertiesStore.saveAllowFnDsa512(0L);
   }
 }