Skip to content

Commit 3fa9b53

Browse files
Federico2014Federico2014
committed
fix(net): dedupe PBFT quorum by recovered SR address
1 parent 9150745 commit 3fa9b53

14 files changed

Lines changed: 222 additions & 55 deletions

File tree

‎actuator/src/main/java/org/tron/core/utils/TransactionUtil.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,13 @@ public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
223223
tswBuilder.setPermission(permission);
224224
if (trx.getSignatureCount() > 0) {
225225
List<ByteString> approveList = new ArrayList<>();
226+
// Read-only introspection: pass osakaAllowed=false so a historical tx
227+
// committed pre-Osaka with a sig above MAX_SIGNATURE_SIZE is still
228+
// resolvable; the upper bound only gates new submissions post-Osaka.
226229
long currentWeight = TransactionCapsule.checkWeight(permission, trx.getSignatureList(),
227230
Sha256Hash.hash(CommonParameter.getInstance()
228231
.isECKeyCryptoEngine(), trx.getRawData().toByteArray()), approveList,
229-
chainBaseManager.getDynamicPropertiesStore());
232+
false);
230233
tswBuilder.addAllApprovedList(approveList);
231234
tswBuilder.setCurrentWeight(currentWeight);
232235
}

‎chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,8 @@ public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
186186
AccountStore accountStore) throws ValidateSignatureException {
187187
try {
188188
ByteString witnessSig = block.getBlockHeader().getWitnessSignature();
189-
if (witnessSig.size() < ChainConstant.MIN_SIGNATURE_SIZE
190-
|| (dynamicPropertiesStore.getAllowTvmOsaka() == 1
191-
&& witnessSig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
189+
if (!SignUtils.isValidLength(witnessSig.size(),
190+
dynamicPropertiesStore.isAllowTvmOsaka())) {
192191
throw new ValidateSignatureException(
193192
"Witness signature size is " + witnessSig.size());
194193
}

‎chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ public static long checkWeight(Permission permission, List<ByteString> sigs, byt
235235
List<ByteString> approveList, DynamicPropertiesStore dynamicPropertiesStore)
236236
throws SignatureException, PermissionException, SignatureFormatException {
237237
Objects.requireNonNull(dynamicPropertiesStore, "dynamicPropertiesStore");
238+
return checkWeight(permission, sigs, hash, approveList,
239+
dynamicPropertiesStore.isAllowTvmOsaka());
240+
}
241+
242+
// Read-only callers (e.g. getTransactionSignWeight) should pass osakaAllowed=false
243+
// so historical transactions with sigs above MAX_SIGNATURE_SIZE are still
244+
// introspectable; the upper bound is meant to gate new submissions post-Osaka.
245+
public static long checkWeight(Permission permission, List<ByteString> sigs, byte[] hash,
246+
List<ByteString> approveList, boolean osakaAllowed)
247+
throws SignatureException, PermissionException, SignatureFormatException {
238248
long currentWeight = 0;
239249
if (sigs.size() > permission.getKeysCount()) {
240250
throw new PermissionException(
@@ -243,9 +253,7 @@ public static long checkWeight(Permission permission, List<ByteString> sigs, byt
243253
}
244254
HashMap addMap = new HashMap();
245255
for (ByteString sig : sigs) {
246-
if (sig.size() < ChainConstant.MIN_SIGNATURE_SIZE
247-
|| (dynamicPropertiesStore.getAllowTvmOsaka() == 1
248-
&& sig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
256+
if (!SignUtils.isValidLength(sig.size(), osakaAllowed)) {
249257
throw new SignatureFormatException("Signature size is " + sig.size());
250258
}
251259
String base64 = TransactionCapsule.getBase64FromByteString(sig);

‎chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3011,6 +3011,10 @@ public void saveAllowTvmOsaka(long value) {
30113011
this.put(ALLOW_TVM_OSAKA, new BytesCapsule(ByteArray.fromLong(value)));
30123012
}
30133013

3014+
public boolean isAllowTvmOsaka() {
3015+
return getAllowTvmOsaka() == 1L;
3016+
}
3017+
30143018
public long getAllowTvmPrague() {
30153019
return Optional.ofNullable(getUnchecked(ALLOW_TVM_PRAGUE))
30163020
.map(BytesCapsule::getData)

‎common/src/main/java/org/tron/core/config/Parameter.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public class ChainConstant {
7070
public static final int SOLIDIFIED_THRESHOLD = 70; // 70%
7171
public static final int PRIVATE_KEY_LENGTH = 64;
7272
public static final int MIN_SIGNATURE_SIZE = 65;
73+
// Canonical ECDSA signature is 65 bytes (r||s||v). 68 = 65 + up to 3 trailing
74+
// padding bytes; this window accommodates historical non-canonical encodings
75+
// observed on chain. Long-term goal is to tighten this back to a strict 65.
7376
public static final int MAX_SIGNATURE_SIZE = 68;
7477
public static final int BLOCK_SIZE = 2_000_000;
7578
public static final long CLOCK_MAX_DELAY = 3600000; // 3600 * 1000 ms

‎consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
import java.util.stream.Collectors;
88
import org.bouncycastle.util.encoders.Hex;
99
import org.tron.common.crypto.ECKey;
10+
import org.tron.common.crypto.SignUtils;
1011
import org.tron.common.overlay.message.Message;
1112
import org.tron.common.utils.ByteUtil;
1213
import org.tron.common.utils.Sha256Hash;
1314
import org.tron.common.utils.StringUtil;
1415
import org.tron.core.capsule.TransactionCapsule;
15-
import org.tron.core.config.Parameter.ChainConstant;
1616
import org.tron.core.exception.P2pException;
1717
import org.tron.core.store.DynamicPropertiesStore;
1818
import org.tron.protos.Protocol.PBFTMessage;
@@ -100,9 +100,8 @@ public DataType getDataType() {
100100
public void analyzeSignature(DynamicPropertiesStore dynamicPropertiesStore)
101101
throws SignatureException {
102102
ByteString signature = getPbftMessage().getSignature();
103-
if (signature.size() < ChainConstant.MIN_SIGNATURE_SIZE
104-
|| (dynamicPropertiesStore.getAllowTvmOsaka() == 1
105-
&& signature.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
103+
if (!SignUtils.isValidLength(signature.size(),
104+
dynamicPropertiesStore.isAllowTvmOsaka())) {
106105
throw new SignatureException("PBFT signature size is " + signature.size());
107106
}
108107
byte[] hash = Sha256Hash.hash(true, getPbftMessage().getRawData().toByteArray());

‎crypto/src/main/java/org/tron/common/crypto/SignUtils.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@
55
import org.tron.common.crypto.ECKey.ECDSASignature;
66
import org.tron.common.crypto.sm2.SM2;
77
import org.tron.common.crypto.sm2.SM2.SM2Signature;
8+
import org.tron.core.config.Parameter.ChainConstant;
89

910
public class SignUtils {
1011

12+
public static boolean isValidLength(int size, boolean osakaAllowed) {
13+
return size >= ChainConstant.MIN_SIGNATURE_SIZE
14+
&& (!osakaAllowed || size <= ChainConstant.MAX_SIGNATURE_SIZE);
15+
}
16+
1117
public static SignInterface getGeneratedRandomSign(
1218
SecureRandom secureRandom, boolean isECKeyCryptoEngine) {
1319
if (isECKeyCryptoEngine) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,10 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
645645
byte[] hash = Sha256Hash.hash(CommonParameter
646646
.getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray());
647647
for (ByteString sig : trx.getSignatureList()) {
648-
if (sig.size() < ChainConstant.MIN_SIGNATURE_SIZE
649-
|| (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
650-
&& sig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
648+
// Read-only introspection: only enforce the lower bound so a tx
649+
// committed pre-Osaka with a sig above MAX_SIGNATURE_SIZE is still
650+
// resolvable; the upper bound only gates new submissions post-Osaka.
651+
if (sig.size() < ChainConstant.MIN_SIGNATURE_SIZE) {
651652
throw new SignatureFormatException(
652653
"Signature size is " + sig.size());
653654
}

‎framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
import org.springframework.beans.factory.annotation.Autowired;
2020
import org.springframework.stereotype.Service;
2121
import org.tron.common.crypto.ECKey;
22+
import org.tron.common.crypto.SignUtils;
2223
import org.tron.common.es.ExecutorServiceManager;
2324
import org.tron.common.utils.ByteArray;
2425
import org.tron.common.utils.Sha256Hash;
2526
import org.tron.consensus.base.Param;
2627
import org.tron.core.ChainBaseManager;
2728
import org.tron.core.capsule.BlockCapsule;
2829
import org.tron.core.capsule.TransactionCapsule;
29-
import org.tron.core.config.Parameter.ChainConstant;
3030
import org.tron.core.db.PbftSignDataStore;
3131
import org.tron.core.exception.P2pException;
3232
import org.tron.core.net.message.TronMessage;
@@ -123,31 +123,28 @@ private void processPBFTCommitMessage(PbftCommitMessage pbftCommitMessage) {
123123
private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
124124
List<ByteString> currentSrList) {
125125
//valid sr list
126+
if (srSignList.size() < Param.getInstance().getAgreeNodeCount()) {
127+
return false;
128+
}
126129
if (srSignList.size() != 0) {
127-
Set<ByteString> srSignSet = new ConcurrentSet();
128-
srSignSet.addAll(srSignList);
129-
if (srSignSet.size() < Param.getInstance().getAgreeNodeCount()) {
130-
logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", srSignSet.size(),
131-
Param.getInstance().getAgreeNodeCount());
132-
return false;
133-
}
134130
byte[] dataHash = Sha256Hash.hash(true, raw.toByteArray());
135131
Set<ByteString> srSet = Sets.newHashSet(currentSrList);
132+
Set<ByteString> validSrAddressSet = new ConcurrentSet();
136133
List<Future<Boolean>> futureList = new ArrayList<>();
137134
for (ByteString sign : srSignList) {
138135
futureList.add(executorService.submit(
139-
new ValidPbftSignTask(raw.getViewN(), srSignSet, dataHash, srSet, sign)));
136+
new ValidPbftSignTask(raw.getViewN(), validSrAddressSet, dataHash, srSet, sign)));
140137
}
141138
for (Future<Boolean> future : futureList) {
142139
try {
143-
if (!future.get()) {
144-
return false;
145-
}
140+
future.get();
146141
} catch (Exception e) {
147142
logger.error("", e);
148143
}
149144
}
150-
if (srSignSet.size() != 0) {
145+
if (validSrAddressSet.size() < Param.getInstance().getAgreeNodeCount()) {
146+
logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", validSrAddressSet.size(),
147+
Param.getInstance().getAgreeNodeCount());
151148
return false;
152149
}
153150
}
@@ -157,15 +154,15 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
157154
private class ValidPbftSignTask implements Callable<Boolean> {
158155

159156
private long viewN;
160-
private Set<ByteString> srSignSet;
157+
private Set<ByteString> validSrAddressSet;
161158
private byte[] dataHash;
162159
private Set<ByteString> srSet;
163160
private ByteString sign;
164161

165-
ValidPbftSignTask(long viewN, Set<ByteString> srSignSet,
162+
ValidPbftSignTask(long viewN, Set<ByteString> validSrAddressSet,
166163
byte[] dataHash, Set<ByteString> srSet, ByteString sign) {
167164
this.viewN = viewN;
168-
this.srSignSet = srSignSet;
165+
this.validSrAddressSet = validSrAddressSet;
169166
this.dataHash = dataHash;
170167
this.srSet = srSet;
171168
this.sign = sign;
@@ -174,10 +171,10 @@ private class ValidPbftSignTask implements Callable<Boolean> {
174171
@Override
175172
public Boolean call() throws Exception {
176173
try {
177-
if (sign.size() < ChainConstant.MIN_SIGNATURE_SIZE
178-
|| (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
179-
&& sign.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
180-
throw new SignatureException("PBFT signature size is " + sign.size());
174+
if (!SignUtils.isValidLength(sign.size(),
175+
chainBaseManager.getDynamicPropertiesStore().isAllowTvmOsaka())) {
176+
logger.error("viewN {} pbft signature size {} is invalid", viewN, sign.size());
177+
return false;
181178
}
182179
byte[] srAddress = ECKey.signatureToAddress(dataHash,
183180
TransactionCapsule.getBase64FromByteString(sign));
@@ -186,7 +183,7 @@ public Boolean call() throws Exception {
186183
ByteArray.toHexString(srAddress));
187184
return false;
188185
}
189-
srSignSet.remove(sign);
186+
validSrAddressSet.add(ByteString.copyFrom(srAddress));
190187
} catch (SignatureException e) {
191188
logger.error("viewN {} valid sr list sign fail!", viewN, e);
192189
return false;

‎framework/src/main/java/org/tron/core/net/service/relay/RelayService.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.tron.common.utils.Sha256Hash;
2525
import org.tron.core.ChainBaseManager;
2626
import org.tron.core.capsule.TransactionCapsule;
27-
import org.tron.core.config.Parameter.ChainConstant;
2827
import org.tron.core.config.args.Args;
2928
import org.tron.core.db.Manager;
3029
import org.tron.core.net.TronNetDelegate;
@@ -154,9 +153,8 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) {
154153
boolean flag;
155154
try {
156155
ByteString signature = msg.getSignature();
157-
if (signature.size() < ChainConstant.MIN_SIGNATURE_SIZE
158-
|| (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
159-
&& signature.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
156+
if (!SignUtils.isValidLength(signature.size(),
157+
chainBaseManager.getDynamicPropertiesStore().isAllowTvmOsaka())) {
160158
return false;
161159
}
162160
Sha256Hash hash = Sha256Hash.of(CommonParameter

0 commit comments

Comments
 (0)