-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat(net): stop tx broadcasting if block cannot solidified #5643
Conversation
if (tronNetDelegate.unsolidifiedBlockCheck()) { | ||
logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID); | ||
return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED) | ||
.setMessage(ByteString.copyFromUtf8("Bock unsolidified.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please optimize the prompt message to make the user aware of the specific reason for the failure, such as too many blocks not being finalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bock unsolidified means this. It would be better to shorten this prompt message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check spelling, Block, not Bock.
|
||
PARAMETER.maxUnsolidifiedBlocks = | ||
config.hasPath(Constant.MAX_UNSOLIDIFIED_BLOCKS) ? config | ||
.getInt(Constant.MAX_UNSOLIDIFIED_BLOCKS) : 1000; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
if (tronNetDelegate.unsolidifiedBlockCheck()) { | ||
logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID); | ||
return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED) | ||
.setMessage(ByteString.copyFromUtf8("Bock unsolidified.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check spelling, Block, not Bock.
long solidNum = chainBaseManager.getSolidBlockId().getNum(); | ||
return headNum - solidNum >= maxUnsolidifiedBlocks; | ||
} | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, done!
1625f0b
to
bf8de17
Compare
@@ -1051,6 +1051,8 @@ message Return { | |||
SERVER_BUSY = 9; | |||
NO_CONNECTION = 10; | |||
NOT_ENOUGH_EFFECTIVE_CONNECTION = 11; | |||
BLOCK_UNSOLIDIFIED = 12; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
What does this PR do?
Stop broadcasting transactions when the block cannot be solidified, refer to issue: #5562
Why are these changes required?
This PR has been tested by:
Follow up
Extra details