Rationale

Really appreciate the contribution of the zkbob team to TRON which optimized the performance of bn128 precompiles #5507 #9. This issue focuses on how to smoothly merge this optimization into the next upgrade.

Historically, new features and on-chain parameter adjustments were often enabled through the proposal, and some minor optimizations (not involving hard forks) were often directly upgraded. However, this time the optimization of bn128 precompiles has greatly improved the performance. Direct upgrade may lead to network forked. However, this optimization is not a governance issue that may not suit through the proposal, so a reasonable upgrade plan needs to be designed.

Here are the various implementation options, along with their pros and cons:

(abandoned) Activate via proposal, reason:

1. This optimization is not a new feature
2. This optimization has nothing to do with the economic model

(abandoned) Activate at the specified block height, reason:

1. Unable to determine the specific block height

(adopted) Activate when major SRs already upgraded by checking ForkController.pass(version), reason:

1. The code changes are few
2. The judgment rules are clear
3. Flexible activate block height, when more than 80% of SRs are upgraded successfully

Implementation

Pseudocode

Take BN128Addition as an example

public static class BN128Addition extends PrecompiledContract {
    ...
    @Override
    public Pair<Boolean, byte[]> execute(byte[] data) {
      ...
      byte[] y2 = parseWord(data, 3);

      if (!ForkController.instance().pass(VERSION_4_8)) {
        // using prev logic when upgrade not completed
        BN128<Fp> p1 = BN128Fp.create(x1, y1);
        if (p1 == null) {
          return Pair.of(false, EMPTY_BYTE_ARRAY);
        }
        BN128<Fp> p2 = BN128Fp.create(x2, y2);
        if (p2 == null) {
          return Pair.of(false, EMPTY_BYTE_ARRAY);
        }
        BN128<Fp> res = p1.add(p2).toEthNotation();
        return Pair.of(true, encodeRes(res.x().bytes(), res.y().bytes()));
      } else {
        // using new logic when upgrade success
        if (!JLibarkworks.libarkworksG1IsValid(x1, y1)) {
          return Pair.of(false, EMPTY_BYTE_ARRAY);
        }
        byte[] p1 = ArrayUtils.addAll(x1, y1);
        if (!JLibarkworks.libarkworksG1IsValid(x2, y2)) {
          return Pair.of(false, EMPTY_BYTE_ARRAY);
        }
        byte[] p2 = ArrayUtils.addAll(x2, y2);
        byte[] res = JLibarkworks.libarkworksAddG1(p1, p2);
        if (res == null) {
          return Pair.of(false, EMPTY_BYTE_ARRAY);
        }
        return Pair.of(true, res);
      }
    }
  }

Testnet

Because this upgrade involves two repos: java-tron and zksnark-java-sdk. Divide the upgrade process into multiple stages, each stage accomplishes its own testing goals:

• Keep java-tron remains the previous version, parts of SRs & fullnodes pack the new version of zksnark-java-sdk.jar for their fullnode.jar: test the compatibility of the previous and new versions of SDK. This can start after v4.7.3 is released.

  • Check the normal transactions works well
  • Check the transactions involving bn128 behave the same as before
  • Better to test for a relatively long time as it is important

• <80% SRs adopt the new version both for java-tron & zksnark-java-sdk: the optimization does not take effect at this time

  • Use the same testing process as the above step
  • Confirm that bn128 type transactions still execute the previous logic code

• >80% SRs adopt the new version: the optimization takes effect

  • Test whether the new logic is effective

• Upgrade the fullnodes, and check their sync status

The current version does not allow downgrade once SRs are upgraded, so the malicious downgrade of some SRs during the upgrade process is not considered.

After v4.7.3 is released, I will promote the merge of bn128 PRs. When the PRs are merged, I will start the development of this feature.