Invalid witness address when the localwitness is null

[Federico2014]

Software Versions

OS : Mac OS
JVM : Oracle Corporation 1.8.0_161 amd64
Version : 4.7.7

Expected behavior

When launching the full-node with the null localwitness config, the node should run normally without an witness address .

localwitness = [
]

Actual behavior

It generates an invalid witness address: 41dcc703c0e500b653ca82273b7bfad8045d85a470 even when the private key is null.

After analysis, I find that even if the localwitness is null, it will still execute initWitnessAccountAddress function.

 else if (config.hasPath(Constant.LOCAL_WITNESS)) {
      localWitnesses = new LocalWitnesses();
      List<String> localwitness = config.getStringList(Constant.LOCAL_WITNESS);
      localWitnesses.setPrivateKeys(localwitness);
      witnessAddressCheck(config);
      localWitnesses.initWitnessAccountAddress(PARAMETER.isECKeyCryptoEngine());
      logger.debug("Got privateKey from config.conf");
    }

From the initWitnessAccountAddress function, it will invoke the fromPrivate function.

  public void initWitnessAccountAddress(boolean isECKeyCryptoEngine) {
    if (witnessAccountAddress == null) {
      byte[] privateKey = ByteArray.fromHexString(getPrivateKey());
      final SignInterface ecKey = SignUtils.fromPrivate(privateKey,
          isECKeyCryptoEngine);
      this.witnessAccountAddress = ecKey.getAddress();
    }
  }

In this case, the privKeyBytes is [] instead of null, so it continues to execute fromPrivate(new BigInteger(1, privKeyBytes)). So, the empty private key is finally passed to the Bouncycastle library, which leads to an implicit error and outputs an invalid address.

  public static ECKey fromPrivate(byte[] privKeyBytes) {
    if (Objects.isNull(privKeyBytes)) {
      return null;
    }
    return fromPrivate(new BigInteger(1, privKeyBytes));
  }

[abc-x-t]

Discovering this question is really great, and looking forward to solving it! No address should be generated when no localwitness is provided.

[Federico2014]

@abc-x-t Yes, we will solve it after discussion.

[halibobo1205]

I also found this issue when upgrading the BCP-related dependencies of org.bouncycastle. See curve25519 private key parsing fails since 1.74.
Passing an empty private key will throw an IllegalArgumentException:

java.lang.IllegalArgumentException: Scalar is not in the interval [1, n - 1]

[Federico2014]

@halibobo1205 Yes, the current BC version 1.69 is incomplete, so the exception is not thrown out explicitly. We should consider upgrading the BC.

[Federico2014]

Currently, the bouncycastle version of java-tron is v1.69, while the libp2p bouncycastle version is v1.79. I have checked the bouncycastle release notes. If we upgrade the bouncycastle version to v1.79, there are some potential benefits:

• optimizations for the Blake3, KeccakDisget, SHA-256 algorithms;
• optimize the key range check of SM2;
• support the post-quantum cryptography, such as BIKE, CMCE, Frodo, HQC, Picnic,LMS/HSS, SPHINCS+, Dilithium, Falcon, and NTRU, which will prepare for future upgrades.
• keep up with libp2p boucycastle version.

@halibobo1205 Do you think is it necessary to upgrade the bouncycastle version?

[halibobo1205]

Upgrade the Bouncycastle version to 1.8.0? @Federico2014

[Federico2014]

I just consider keeping the same version with libp2p. Upgrading to version 1.80 is ok.

[abc-x-t]

keeping the same version with libp2p

Yes, I think keep same version with libp2p is better, because we are not sure whether higher version is compatible with libp2p