Skip to content

Commit 8d20e0e

Browse files
Federico2014Federico2014
committed
feat(crypto): shielded transaction API security enhancement
1 parent 208807d commit 8d20e0e

13 files changed

Lines changed: 334 additions & 214 deletions

File tree

‎common/src/main/java/org/tron/common/parameter/CommonParameter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ public class CommonParameter {
335335

336336
@Getter
337337
@Setter
338-
public boolean allowShieldedTransactionApi; // clearParam: true
338+
public boolean allowShieldedTransactionApi; // clearParam: false
339339
@Getter
340340
@Setter
341341
public long blockNumForEnergyLimit;

‎framework/src/main/java/org/tron/core/Wallet.java‎

Lines changed: 206 additions & 200 deletions
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ public static void applyConfigParams(
726726
logger.warn("Configuring [node.fullNodeAllowShieldedTransaction] will be deprecated. "
727727
+ "Please use [node.allowShieldedTransactionApi] instead.");
728728
} else {
729-
PARAMETER.allowShieldedTransactionApi = true;
729+
PARAMETER.allowShieldedTransactionApi = false;
730730
}
731731

732732
PARAMETER.zenTokenId = config.hasPath(ConfigKey.NODE_ZEN_TOKENID)

‎framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.tron.api.GrpcAPI;
1515
import org.tron.api.GrpcAPI.BytesMessage;
1616
import org.tron.api.GrpcAPI.ShieldedTRC20Parameters;
17+
import org.tron.common.math.StrictMathWrapper;
1718
import org.tron.common.utils.ByteArray;
1819
import org.tron.common.utils.ByteUtil;
1920
import org.tron.common.utils.Sha256Hash;
@@ -547,8 +548,8 @@ public void addSpend(
547548
byte[] anchor,
548549
byte[] path,
549550
long position) throws ZksnarkException {
551+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
550552
spends.add(new SpendDescriptionInfo(expsk, note, anchor, path, position));
551-
valueBalance += note.getValue();
552553
}
553554

554555
public void addSpend(
@@ -558,8 +559,8 @@ public void addSpend(
558559
byte[] anchor,
559560
byte[] path,
560561
long position) {
562+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
561563
spends.add(new SpendDescriptionInfo(expsk, note, alpha, anchor, path, position));
562-
valueBalance += note.getValue();
563564
}
564565

565566
public void addSpend(
@@ -570,23 +571,23 @@ public void addSpend(
570571
byte[] anchor,
571572
byte[] path,
572573
long position) {
574+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
573575
spends.add(new SpendDescriptionInfo(ak, nsk, note, alpha, anchor, path, position));
574-
valueBalance += note.getValue();
575576
}
576577

577578
public void addOutput(byte[] ovk, PaymentAddress to, long value, byte[] memo)
578579
throws ZksnarkException {
579580
Note note = new Note(to, value);
580581
note.setMemo(memo);
582+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
581583
receives.add(new ReceiveDescriptionInfo(ovk, note));
582-
valueBalance -= value;
583584
}
584585

585586
public void addOutput(byte[] ovk, DiversifierT d, byte[] pkD, long value, byte[] r, byte[] memo) {
586587
Note note = new Note(d, pkD, value, r);
587588
note.setMemo(memo);
589+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
588590
receives.add(new ReceiveDescriptionInfo(ovk, note));
589-
valueBalance -= value;
590591
}
591592

592593
public static class SpendDescriptionInfo {

‎framework/src/main/java/org/tron/core/zen/ZenTransactionBuilder.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import lombok.Setter;
1111
import lombok.extern.slf4j.Slf4j;
1212
import org.apache.commons.lang3.ArrayUtils;
13+
import org.tron.common.math.StrictMathWrapper;
1314
import org.tron.common.utils.ByteArray;
1415
import org.tron.common.zksnark.IncrementalMerkleVoucherContainer;
1516
import org.tron.common.zksnark.JLibrustzcash;
@@ -66,17 +67,17 @@ public ZenTransactionBuilder() {
6667
}
6768

6869
public void addSpend(SpendDescriptionInfo spendDescriptionInfo) {
70+
valueBalance = StrictMathWrapper.addExact(valueBalance, spendDescriptionInfo.note.getValue());
6971
spends.add(spendDescriptionInfo);
70-
valueBalance += spendDescriptionInfo.note.getValue();
7172
}
7273

7374
public void addSpend(
7475
ExpandedSpendingKey expsk,
7576
Note note,
7677
byte[] anchor,
7778
IncrementalMerkleVoucherContainer voucher) throws ZksnarkException {
79+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
7880
spends.add(new SpendDescriptionInfo(expsk, note, anchor, voucher));
79-
valueBalance += note.getValue();
8081
}
8182

8283
public void addSpend(
@@ -85,8 +86,8 @@ public void addSpend(
8586
byte[] alpha,
8687
byte[] anchor,
8788
IncrementalMerkleVoucherContainer voucher) {
89+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
8890
spends.add(new SpendDescriptionInfo(expsk, note, alpha, anchor, voucher));
89-
valueBalance += note.getValue();
9091
}
9192

9293
public void addSpend(
@@ -97,23 +98,23 @@ public void addSpend(
9798
byte[] alpha,
9899
byte[] anchor,
99100
IncrementalMerkleVoucherContainer voucher) {
101+
valueBalance = StrictMathWrapper.addExact(valueBalance, note.getValue());
100102
spends.add(new SpendDescriptionInfo(ak, nsk, ovk, note, alpha, anchor, voucher));
101-
valueBalance += note.getValue();
102103
}
103104

104105
public void addOutput(byte[] ovk, PaymentAddress to, long value, byte[] memo)
105106
throws ZksnarkException {
106107
Note note = new Note(to, value);
107108
note.setMemo(memo);
109+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
108110
receives.add(new ReceiveDescriptionInfo(ovk, note));
109-
valueBalance -= value;
110111
}
111112

112113
public void addOutput(byte[] ovk, DiversifierT d, byte[] pkD, long value, byte[] r, byte[] memo) {
113114
Note note = new Note(d, pkD, value, r);
114115
note.setMemo(memo);
116+
valueBalance = StrictMathWrapper.subtractExact(valueBalance, value);
115117
receives.add(new ReceiveDescriptionInfo(ovk, note));
116-
valueBalance -= value;
117118
}
118119

119120
public void setTransparentInput(byte[] address, long value) {

‎framework/src/main/resources/config.conf‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,13 @@ node {
178178

179179
minParticipationRate = 15
180180

181-
# allowShieldedTransactionApi = true
181+
# WARNING: Some shielded transaction APIs require sending private keys as parameters.
182+
# Calling these APIs on untrusted or remote nodes may leak your private keys.
183+
# It is recommended to invoke them locally for development and testing.
184+
# To opt in, set: allowShieldedTransactionApi = true
185+
# Migration: the legacy key node.fullNodeAllowShieldedTransaction is still supported
186+
# but deprecated; please migrate to node.allowShieldedTransactionApi.
187+
# allowShieldedTransactionApi = false
182188

183189
# openPrintLog = true
184190

‎framework/src/test/java/org/tron/core/ShieldedTRC20BuilderTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.apache.commons.lang3.ArrayUtils;
1010
import org.apache.commons.lang3.tuple.Pair;
1111
import org.bouncycastle.util.encoders.Hex;
12+
import org.junit.AfterClass;
1213
import org.junit.Assert;
1314
import org.junit.BeforeClass;
1415
import org.junit.Ignore;
@@ -73,9 +74,18 @@ public class ShieldedTRC20BuilderTest extends BaseTest {
7374
VerifyTransferProof transferContract = new VerifyTransferProof();
7475
VerifyBurnProof burnContract = new VerifyBurnProof();
7576

77+
private static boolean origShieldedApi;
78+
7679
@BeforeClass
7780
public static void initZksnarkParams() {
7881
ZksnarkInitService.librustzcashInitZksnarkParams();
82+
origShieldedApi = Args.getInstance().allowShieldedTransactionApi;
83+
Args.getInstance().allowShieldedTransactionApi = true;
84+
}
85+
86+
@AfterClass
87+
public static void restoreShieldedApi() {
88+
Args.getInstance().allowShieldedTransactionApi = origShieldedApi;
7989
}
8090

8191
@Ignore

‎framework/src/test/java/org/tron/core/actuator/ShieldedTransferActuatorTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,9 @@ public void publicAddressToShieldNoteValueFailure() {
11571157
actuator.validate();
11581158
actuator.execute(ret);
11591159
Assert.assertTrue(false);
1160+
} catch (ArithmeticException e) {
1161+
// StrictMathWrapper.subtractExact throws ArithmeticException on overflow
1162+
Assert.assertTrue(true);
11601163
} catch (ContractValidateException e) {
11611164
Assert.assertTrue(e instanceof ContractValidateException);
11621165
Assert.assertEquals("librustzcashSaplingFinalCheck error", e.getMessage());
@@ -1346,5 +1349,58 @@ public void shieldAddressToPublic() {
13461349
Assert.assertTrue(false);
13471350
}
13481351
}
1352+
1353+
/**
1354+
* Test that shielded transfer transaction validation works even when
1355+
* allowShieldedTransactionApi is disabled. This verifies that the API flag
1356+
* only gates wallet/helper APIs, not the core transaction validation logic.
1357+
*/
1358+
@Test
1359+
public void shieldedTransferValidationWorksWhenApiDisabled() {
1360+
boolean orig = Args.getInstance().isAllowShieldedTransactionApi();
1361+
// Disable the shielded API (this should NOT affect transaction validation)
1362+
Args.getInstance().setAllowShieldedTransactionApi(false);
1363+
1364+
dbManager.getDynamicPropertiesStore().saveAllowShieldedTransaction(1);
1365+
dbManager.getDynamicPropertiesStore().saveTotalShieldedPoolValue(AMOUNT);
1366+
1367+
try {
1368+
ZenTransactionBuilder builder = new ZenTransactionBuilder(wallet);
1369+
SpendingKey sk = SpendingKey.random();
1370+
ExpandedSpendingKey expsk = sk.expandedSpendingKey();
1371+
PaymentAddress address = sk.defaultAddress();
1372+
Note note = new Note(address, AMOUNT);
1373+
IncrementalMerkleVoucherContainer voucher = createSimpleMerkleVoucherContainer(note.cm());
1374+
byte[] anchor = voucher.root().getContent().toByteArray();
1375+
dbManager.getMerkleContainer()
1376+
.putMerkleTreeIntoStore(anchor, voucher.getVoucherCapsule().getTree());
1377+
builder.addSpend(expsk, note, anchor, voucher);
1378+
1379+
addZeroValueOutputNote(builder);
1380+
1381+
long fee = dbManager.getDynamicPropertiesStore().getShieldedTransactionCreateAccountFee();
1382+
String addressNotExist =
1383+
Wallet.getAddressPreFixString() + "8ba2aaae540c642e44e3bed5522c63bbc21f0000";
1384+
1385+
builder.setTransparentOutput(ByteArray.fromHexString(addressNotExist), AMOUNT - fee);
1386+
1387+
TransactionCapsule transactionCap = builder.build();
1388+
Contract contract =
1389+
transactionCap.getInstance().toBuilder().getRawDataBuilder().getContract(0);
1390+
ShieldedTransferActuator actuator = new ShieldedTransferActuator();
1391+
actuator.setChainBaseManager(dbManager.getChainBaseManager()).setContract(contract)
1392+
.setTx(transactionCap);
1393+
1394+
// Validation should succeed even when API is disabled
1395+
actuator.validate();
1396+
} catch (ContractValidateException e) {
1397+
Assert.fail("Shielded transfer validation should not throw ContractValidateException: "
1398+
+ e.getMessage());
1399+
} catch (Exception e) {
1400+
Assert.fail("Shielded transfer should not throw Exception: " + e.getMessage());
1401+
} finally {
1402+
Args.getInstance().setAllowShieldedTransactionApi(orig);
1403+
}
1404+
}
13491405
}
13501406

‎framework/src/test/java/org/tron/core/config/args/ArgsTest.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,5 +360,14 @@ public void testConfigStorageDefaults() {
360360

361361
Args.clearParam();
362362
}
363+
364+
@Test
365+
public void testAllowShieldedTransactionApiDefault() {
366+
Args.setParam(new String[]{}, TestConstants.TEST_CONF);
367+
Assert.assertFalse(Args.getInstance().isAllowShieldedTransactionApi());
368+
Args.getInstance().setAllowShieldedTransactionApi(true);
369+
Assert.assertTrue(Args.getInstance().isAllowShieldedTransactionApi());
370+
Args.getInstance().setAllowShieldedTransactionApi(false);
371+
}
363372
}
364373

‎framework/src/test/java/org/tron/core/services/RpcApiServicesTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ public class RpcApiServicesTest {
150150
public static void init() throws IOException {
151151
Args.setParam(new String[] {"-d", temporaryFolder.newFolder().toString()},
152152
TestConstants.TEST_CONF);
153+
getInstance().allowShieldedTransactionApi = true;
153154
Assert.assertEquals(5, getInstance().getRpcMaxRstStream());
154155
Assert.assertEquals(10, getInstance().getRpcSecondsPerWindow());
155156
String OWNER_ADDRESS = Wallet.getAddressPreFixString()

0 commit comments

Comments
 (0)