feat(vm): implement TIP-7939 CLZ opcode#6656
feat(vm): implement TIP-7939 CLZ opcode#6656yanghang8612 wants to merge 1 commit intotronprotocol:developfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if (clz == 256) { | ||
| program.stackPush(new DataWord(256)); | ||
| } else { | ||
| program.stackPush(DataWord.of((byte) clz)); |
There was a problem hiding this comment.
Nice work overall! One minor suggestion: since DataWord.of(byte num) directly assigns bb[31] = num, casting clz to (byte) when it's in [128, 255] introduces a subtle signed/unsigned ambiguity. Would it be worth simplifying both branches into one using new DataWord(int), which handles the full range cleanly?
There was a problem hiding this comment.
Thanks for the review! The (byte) cast here is safe — DataWord.of(byte) assigns the raw bit pattern to bb[31], and DataWord.value() reads it back as unsigned via new BigInteger(1, data). This is consistent with how byte arrays are used throughout the codebase (e.g., DataWord.of((byte) 0xFF) correctly represents 255). The split avoids the double ByteBuffer.allocate overhead in DataWord(int) for the common non-zero path.
| Assert.assertEquals(new DataWord(255), program.getStack().pop()); | ||
|
|
||
| // Verify energy cost = LOW_TIER(5) + PUSH32 cost(3) = 8 | ||
| Assert.assertEquals(8, program.getResult().getEnergyUsed()); |
There was a problem hiding this comment.
The test coverage is really solid — the chosen vectors (0, 1, 255, 256, boundary bits) cover the most critical scenarios nicely! 🎯
One thing that might be worth adding: a couple of test cases where the CLZ result falls in the 128-255 range (e.g., byte[16]=0x01 → CLZ=128, byte[30]=0x01 → CLZ=247). This would explicitly exercise the (byte) cast path for values above 127, giving extra confidence in the signed/unsigned handling. Would make the already-great test suite even more bulletproof!
| // Verify energy cost = LOW_TIER(5) + PUSH32 cost(3) = 8 | ||
| Assert.assertEquals(8, program.getResult().getEnergyUsed()); | ||
|
|
||
| VMConfig.initAllowTvmOsaka(0); |
There was a problem hiding this comment.
Nice work on properly resetting VMConfig.initAllowTvmOsaka(0) in cleanup — shows good testing discipline!
Would it be worth adding a small negative test to verify that CLZ is correctly rejected when allowTvmOsaka is disabled? This would serve as a safety net to ensure the feature gate can never be accidentally bypassed. Just a suggestion to make the gating even more robust 🙂
5 participants
Summary
Implement TIP-7939 CLZ opcode (
0x1e), gated behind the existingallowTvmOsakaflag.LOW_TIER, matching MUL)TRON_V1_5version withappendOsakaOperationsto support Osaka opcodesRelated: tronprotocol/tips#838