Skip to content

feat(net): stop tx broadcasting if block cannot solidified #5643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,14 @@ public class CommonParameter {
@Setter
public long allowCancelAllUnfreezeV2;

@Getter
@Setter
public boolean unsolidifiedBlockCheck;

@Getter
@Setter
public int maxUnsolidifiedBlocks;

private static double calcMaxTimeRatio() {
//return max(2.0, min(5.0, 5 * 4.0 / max(Runtime.getRuntime().availableProcessors(), 1)));
return 5.0;
Expand Down
4 changes: 4 additions & 0 deletions common/src/main/java/org/tron/core/Constant.java
Original file line number Diff line number Diff line change
Expand Up @@ -371,4 +371,8 @@ public class Constant {
public static final String DYNAMIC_CONFIG_CHECK_INTERVAL = "node.dynamicConfig.checkInterval";

public static final String COMMITTEE_ALLOW_TVM_SHANGHAI = "committee.allowTvmShangHai";

public static final String UNSOLIDIFIED_BLOCK_CHECK = "node.unsolidifiedBlockCheck";

public static final String MAX_UNSOLIDIFIED_BLOCKS = "node.maxUnsolidifiedBlocks";
}
8 changes: 8 additions & 0 deletions framework/src/main/java/org/tron/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,14 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) {
Sha256Hash txID = trx.getTransactionId();
try {
TransactionMessage message = new TransactionMessage(signedTransaction.toByteArray());

if (tronNetDelegate.isBlockUnsolidified()) {
logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID);
return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED)
.setMessage(ByteString.copyFromUtf8("Block unsolidified."))
.build();
}

if (minEffectiveConnection != 0) {
if (tronNetDelegate.getActivePeer().isEmpty()) {
logger.warn("Broadcast transaction {} has failed, no connection.", txID);
Expand Down
10 changes: 10 additions & 0 deletions framework/src/main/java/org/tron/core/config/args/Args.java
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ public static void clearParam() {
PARAMETER.dynamicConfigEnable = false;
PARAMETER.dynamicConfigCheckInterval = 600;
PARAMETER.allowTvmShangHai = 0;
PARAMETER.unsolidifiedBlockCheck = true;
PARAMETER.maxUnsolidifiedBlocks = 1000;
}

/**
Expand Down Expand Up @@ -1181,6 +1183,14 @@ public static void setParam(final String[] args, final String confFileName) {
config.hasPath(Constant.COMMITTEE_ALLOW_TVM_SHANGHAI) ? config
.getInt(Constant.COMMITTEE_ALLOW_TVM_SHANGHAI) : 0;

PARAMETER.unsolidifiedBlockCheck =
!config.hasPath(Constant.UNSOLIDIFIED_BLOCK_CHECK)
|| config.getBoolean(Constant.UNSOLIDIFIED_BLOCK_CHECK);

PARAMETER.maxUnsolidifiedBlocks =
config.hasPath(Constant.MAX_UNSOLIDIFIED_BLOCKS) ? config
.getInt(Constant.MAX_UNSOLIDIFIED_BLOCKS) : 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value is determined how?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to the issue test data, I personally think 1000 is OK.


logConfig();
}

Expand Down
15 changes: 15 additions & 0 deletions framework/src/main/java/org/tron/core/net/TronNetDelegate.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.tron.core.capsule.BlockCapsule.BlockId;
import org.tron.core.capsule.PbftSignCapsule;
import org.tron.core.capsule.TransactionCapsule;
import org.tron.core.config.args.Args;
import org.tron.core.db.Manager;
import org.tron.core.exception.AccountResourceInsufficientException;
import org.tron.core.exception.BadBlockException;
Expand Down Expand Up @@ -98,6 +99,11 @@ public class TronNetDelegate {
@Setter
private volatile boolean exit = true;

private int maxUnsolidifiedBlocks = Args.getInstance().getMaxUnsolidifiedBlocks();

private boolean unsolidifiedBlockCheck
= Args.getInstance().isUnsolidifiedBlockCheck();

private Cache<BlockId, Long> freshBlockId = CacheBuilder.newBuilder()
.maximumSize(blockIdCacheSize).expireAfterWrite(1, TimeUnit.HOURS)
.recordStats().build();
Expand Down Expand Up @@ -365,4 +371,13 @@ public long getMaintenanceTimeInterval() {
return chainBaseManager.getDynamicPropertiesStore().getMaintenanceTimeInterval();
}

public boolean isBlockUnsolidified() {
if (!unsolidifiedBlockCheck) {
return false;
}
long headNum = chainBaseManager.getHeadBlockNum();
long solidNum = chainBaseManager.getSolidBlockId().getNum();
return headNum - solidNum >= maxUnsolidifiedBlocks;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename the method unsolidifiedBlockCheck. It should not be the same as varible, it is confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, done!

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public void processMessage(PeerConnection peer, TronMessage msg) {
}

private boolean check(PeerConnection peer, InventoryMessage inventoryMessage) {

InventoryType type = inventoryMessage.getInventoryType();
int size = inventoryMessage.getHashList().size();

Expand All @@ -52,6 +53,12 @@ private boolean check(PeerConnection peer, InventoryMessage inventoryMessage) {
return false;
}

if (type.equals(InventoryType.TRX) && tronNetDelegate.isBlockUnsolidified()) {
logger.warn("Drop inv: {} size: {} from Peer {}, block unsolidified",
type, size, peer.getInetAddress());
return false;
}

if (type.equals(InventoryType.TRX) && transactionsMsgHandler.isBusy()) {
logger.warn("Drop inv: {} size: {} from Peer {}, transactionsMsgHandler is busy",
type, size, peer.getInetAddress());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ public void get() {
Assert.assertEquals(0, parameter.getActiveNodes().size());
Assert.assertEquals(30, parameter.getMaxConnections());
Assert.assertEquals(43, parameter.getNodeP2pVersion());
Assert.assertEquals(1000, parameter.getMaxUnsolidifiedBlocks());
Assert.assertEquals(true, parameter.isUnsolidifiedBlockCheck());
//Assert.assertEquals(30, args.getSyncNodeCount());

// gRPC network configs checking
Expand Down
51 changes: 51 additions & 0 deletions framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package org.tron.core.net;

import static org.mockito.Mockito.mock;

import java.lang.reflect.Field;
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
import org.tron.common.parameter.CommonParameter;
import org.tron.common.utils.Sha256Hash;
import org.tron.core.ChainBaseManager;
import org.tron.core.Constant;
import org.tron.core.capsule.BlockCapsule;
import org.tron.core.config.args.Args;

public class TronNetDelegateTest {

@Test
public void test() throws Exception {
Args.setParam(new String[] {"-w"}, Constant.TEST_CONF);
CommonParameter parameter = Args.getInstance();
Args.logConfig();

BlockCapsule.BlockId blockId = new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, 10000L);

TronNetDelegate tronNetDelegate = new TronNetDelegate();

ChainBaseManager chainBaseManager = mock(ChainBaseManager.class);
Mockito.when(chainBaseManager.getHeadBlockNum()).thenReturn(10000L);
Mockito.when(chainBaseManager.getSolidBlockId()).thenReturn(blockId);

Field field = tronNetDelegate.getClass().getDeclaredField("chainBaseManager");
field.setAccessible(true);
field.set(tronNetDelegate, chainBaseManager);

Assert.assertTrue(!tronNetDelegate.isBlockUnsolidified());

blockId = new BlockCapsule.BlockId(Sha256Hash.ZERO_HASH, 1L);
Mockito.when(chainBaseManager.getSolidBlockId()).thenReturn(blockId);
Assert.assertTrue(tronNetDelegate.isBlockUnsolidified());

parameter.setUnsolidifiedBlockCheck(false);
tronNetDelegate = new TronNetDelegate();

field = tronNetDelegate.getClass().getDeclaredField("unsolidifiedBlockCheck");
field.setAccessible(true);
field.set(tronNetDelegate, false);

Assert.assertTrue(!tronNetDelegate.isBlockUnsolidified());
}
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
package org.tron.core.net.messagehandler;

import static org.mockito.Mockito.mock;

import java.lang.reflect.Field;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import org.junit.Test;
import org.mockito.Mockito;
import org.tron.core.Constant;
import org.tron.core.config.args.Args;
import org.tron.core.net.TronNetDelegate;
import org.tron.core.net.message.adv.InventoryMessage;
import org.tron.core.net.peer.PeerConnection;
import org.tron.p2p.connection.Channel;
import org.tron.protos.Protocol.Inventory.InventoryType;

public class InventoryMsgHandlerTest {

private InventoryMsgHandler handler = new InventoryMsgHandler();

@Test
public void testProcessMessage() throws Exception {
InventoryMsgHandler handler = new InventoryMsgHandler();
Args.setParam(new String[] {"-w"}, Constant.TEST_CONF);
Args.logConfig();

InventoryMessage msg = new InventoryMessage(new ArrayList<>(), InventoryType.TRX);
PeerConnection peer = new PeerConnection();
peer.setChannel(getChannel("1.0.0.3", 1000));
Expand All @@ -31,6 +39,16 @@ public void testProcessMessage() throws Exception {
peer.setNeedSyncFromUs(true);
handler.processMessage(peer, msg);

peer.setNeedSyncFromUs(false);

TronNetDelegate tronNetDelegate = mock(TronNetDelegate.class);
Mockito.when(tronNetDelegate.isBlockUnsolidified()).thenReturn(true);

Field field = handler.getClass().getDeclaredField("tronNetDelegate");
field.setAccessible(true);
field.set(handler, tronNetDelegate);

handler.processMessage(peer, msg);
}

private Channel getChannel(String host, int port) throws Exception {
Expand Down
2 changes: 2 additions & 0 deletions protocol/src/main/protos/api/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,8 @@ message Return {
SERVER_BUSY = 9;
NO_CONNECTION = 10;
NOT_ENOUGH_EFFECTIVE_CONNECTION = 11;
BLOCK_UNSOLIDIFIED = 12;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be the reason TOO_MANY_BLOCK_UNSOLIDIFIED more clearful ? We generally believe that less than a certain number of unsodlified blocks is normal. It's tiny.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Briefly describe what happened, too many doesn't make much sense.


OTHER_ERROR = 20;
}

Expand Down