Skip to content
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.

Sign up for GitHub

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

3s consensus - With consensus time included into consensus intervals. #3622

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
+228 −37
Open

3s consensus - With consensus time included into consensus intervals. #3622

Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
b5fd4ec
First commit and no format
vncoelho Dec 5, 2024
91c4bd2
Start adjusting param but devcontainer not working
vncoelho Dec 5, 2024
9ba85d8
Merge branch 'master' into decreaseblocktime-3s
vncoelho Dec 6, 2024
a095496
Merge branch 'master' into decreaseblocktime-3s
Jim8y Dec 6, 2024
3329c54
Update docs/config.json.md
vncoelho Dec 7, 2024
774471a
Merge branch 'master' into decreaseblocktime-3s
vncoelho Dec 9, 2024
739beaf
format
vncoelho Dec 9, 2024
af42801
Merge branch 'master' into decreaseblocktime-3s
vncoelho Dec 10, 2024
9d677c5
Merge branch 'master' into decreaseblocktime-3s
shargon Dec 12, 2024
cd91440
Update src/Neo.CLI/config.fs.mainnet.json
shargon Dec 12, 2024
26b1e58
Update src/Neo.CLI/config.json
shargon Dec 12, 2024
3b52a3c
Update src/Neo.CLI/config.mainnet.json
shargon Dec 12, 2024
f016575
include the consensus time into block intervals.
Jim8y Dec 12, 2024
1570990
Merge branch 'master' into 3s-consensus
Jim8y Dec 16, 2024
6fe0abf
Merge branch 'master' into 3s-consensus
Jim8y Feb 8, 2025
19d0850
fix ut
Jim8y Feb 8, 2025
18cd445
update gasperblock, and ut is expected to fail due to hardfork settings.
Jim8y Feb 8, 2025
169a7e7
update the code and unit tests
Jim8y Feb 9, 2025
8b47891
Merge branch 'master' into 3s-consensus
Jim8y Feb 10, 2025
45eb20d
Merge branch 'master' into 3s-consensus
Jim8y Feb 11, 2025
ded7002
apply shargon's suggestion and avoid change the storage.
Jim8y Feb 11, 2025
d9c635c
Merge branch '3s-consensus' of github.com:Jim8y/neo into 3s-consensus
Jim8y Feb 11, 2025
6bc2770
revert change to maxtransactionsperblock
Jim8y Feb 11, 2025
1c2dcd1
Update docs/config.json.md
Jim8y Feb 11, 2025
6def9c8
Update src/Neo.CLI/config.testnet.json
Jim8y Feb 11, 2025
3664890
Update tests/Neo.Plugins.OracleService.Tests/config.json
Jim8y Feb 11, 2025
fde030c
Merge branch 'master' into 3s-consensus
Jim8y Feb 12, 2025
ae6594c
Merge branch 'master' into 3s-consensus
vncoelho Feb 12, 2025
2a39868
Merge branch 'master' into 3s-consensus
Jim8y Feb 13, 2025
593e573
Merge branch 'master' into 3s-consensus
Jim8y Feb 19, 2025
430f346
Merge branch 'master' into 3s-consensus
Jim8y Feb 25, 2025
89891b7
Merge branch 'master' into 3s-consensus
vncoelho Feb 26, 2025
e81cd4c
Merge branch 'master' into 3s-consensus
shargon Feb 27, 2025
59b1d17
Merge branch 'master' into 3s-consensus
Jim8y Mar 4, 2025
7ea21f1
update to policy contract
Jim8y Mar 4, 2025
6e5686c
add doc
Jim8y Mar 4, 2025
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
2 changes: 1 addition & 1 deletion benchmarks/Neo.Benchmarks/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"ProtocolConfiguration": {
"Network": 860833102,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 512,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
Expand Down
2 changes: 1 addition & 1 deletion docs/config.json.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ This README provides an explanation for each field in the JSON configuration fil
- **AddressVersion**: Version byte used in Neo address generation. Default is `53`.

### MillisecondsPerBlock
- **MillisecondsPerBlock**: Time interval between blocks in milliseconds. Default is `15000` (15 seconds).
- **MillisecondsPerBlock**: Time interval between blocks in milliseconds. Default is `3000` (3 seconds).

### MaxTransactionsPerBlock
- **MaxTransactionsPerBlock**: Maximum number of transactions allowed per block. Default is `512`.
Expand Down
2 changes: 1 addition & 1 deletion src/Neo.CLI/config.fs.testnet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"ProtocolConfiguration": {
"Network": 91466898,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 512,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 17280,
Expand Down
2 changes: 1 addition & 1 deletion src/Neo.CLI/config.testnet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"ProtocolConfiguration": {
"Network": 894710606,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 5000,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
Expand Down
4 changes: 2 additions & 2 deletions src/Neo/ProtocolSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public record ProtocolSettings
/// <summary>
/// The maximum increment of the <see cref="Transaction.ValidUntilBlock"/> field.
/// </summary>
public uint MaxValidUntilBlockIncrement => 86400000 / MillisecondsPerBlock;
public uint MaxValidUntilBlockIncrement => 86400000 / MillisecondsPerBlock; //TODO keep the same??

/// <summary>
/// Indicates the maximum number of transactions that can be contained in a block.
Expand Down Expand Up @@ -114,7 +114,7 @@ public record ProtocolSettings
StandbyCommittee = Array.Empty<ECPoint>(),
ValidatorsCount = 0,
SeedList = Array.Empty<string>(),
MillisecondsPerBlock = 15000,
MillisecondsPerBlock = 3000,
MaxTransactionsPerBlock = 512,
MemoryPoolMaxTransactions = 50_000,
MaxTraceableBlocks = 2_102_400,
Expand Down
4 changes: 2 additions & 2 deletions src/Plugins/DBFTPlugin/Consensus/ConsensusService.Check.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ private bool CheckPrepareResponse()
}

// Timeout extension due to prepare response sent
// around 2*15/M=30.0/5 ~ 40% block time (for M=5)
ExtendTimerByFactor(2);
// around 4*3/M=12.0/5 ~ 80% block time (for M=5)
ExtendTimerByFactor(4);

Log($"Sending {nameof(PrepareResponse)}");
localNode.Tell(new LocalNode.SendDirectly { Inventory = context.MakePrepareResponse() });
Expand Down
15 changes: 9 additions & 6 deletions src/Plugins/DBFTPlugin/Consensus/ConsensusService.OnMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,11 @@ private void OnPrepareRequestReceived(ExtensiblePayload payload, PrepareRequest
}

// Timeout extension: prepare request has been received with success
// around 2*15/M=30.0/5 ~ 40% block time (for M=5)
ExtendTimerByFactor(2);
// around 4*3/M=12.0/5 ~ 80% block time (for M=5)
ExtendTimerByFactor(4);

onPrepareReceivedTime = TimeProvider.Current.UtcNow;
onPrepareBlockIndex = message.BlockIndex;

context.Block.Header.Timestamp = message.Timestamp;
context.Block.Header.Nonce = message.Nonce;
Expand Down Expand Up @@ -171,8 +174,8 @@ private void OnPrepareResponseReceived(ExtensiblePayload payload, PrepareRespons
return;

// Timeout extension: prepare response has been received with success
// around 2*15/M=30.0/5 ~ 40% block time (for M=5)
ExtendTimerByFactor(2);
// around 4*3/M=12.0/5 ~ 80% block time (for M=5)
ExtendTimerByFactor(4);

Log($"{nameof(OnPrepareResponseReceived)}: height={message.BlockIndex} view={message.ViewNumber} index={message.ValidatorIndex}");
context.PreparationPayloads[message.ValidatorIndex] = payload;
Expand Down Expand Up @@ -210,8 +213,8 @@ private void OnCommitReceived(ExtensiblePayload payload, Commit commit)
if (commit.ViewNumber == context.ViewNumber)
{
// Timeout extension: commit has been received with success
// around 4*15s/M=60.0s/5=12.0s ~ 80% block time (for M=5)
ExtendTimerByFactor(4);
// around 6*3s/M=18.0s/5=12.0s ~ 120% block time (for M=5)
ExtendTimerByFactor(6);

Log($"{nameof(OnCommitReceived)}: height={commit.BlockIndex} view={commit.ViewNumber} index={commit.ValidatorIndex} nc={context.CountCommitted} nf={context.CountFailed}");

Expand Down
8 changes: 6 additions & 2 deletions src/Plugins/DBFTPlugin/Consensus/ConsensusService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ private class Timer { public uint Height; public byte ViewNumber; }
private readonly IActorRef blockchain;
private ICancelable timer_token;
private DateTime block_received_time;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be dropped now.

private DateTime onPrepareReceivedTime;
private uint onPrepareBlockIndex;
private uint block_received_index;
private bool started = false;

Expand Down Expand Up @@ -94,9 +96,11 @@ private void InitializeConsensus(byte viewNumber)
else
{
TimeSpan span = neoSystem.Settings.TimePerBlock;
if (block_received_index + 1 == context.Block.Index)

if (block_received_index + 1 == context.Block.Index && onPrepareBlockIndex + 1 == context.Block.Index)
{
var diff = TimeProvider.Current.UtcNow - block_received_time;
// Include the consensus time into the consensus intervals.
var diff = TimeProvider.Current.UtcNow - onPrepareReceivedTime;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the major change to this pr

Copy link
Member

Choose a reason for hiding this comment

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

This should be in another PR and not here, if that is the case.

And for your notice, as I said in our meetings, this configuration has been tested in the past.

I think that you need more experimental tests before publishing things to quick and even writing new standards (that are not even necessary).
I am not saying that the change is terrible, @Jim8y, but there are some consequences, I think that you did not analyse the entire scenario before pushing this and creating standards with it.

I will surely be against this change here in this PR for a series of reasons.
Try this in a decentralized scenario or inserting delays (you can deal with that on docker) or create on AWS/AZURE in different locations.

Copy link
Member

Choose a reason for hiding this comment

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

this configuration has been tested in the past.

Lots have changed in code since then. Would that affect anything like this? if so, we have to run these tests again to ensure its trueful.

Copy link
Contributor Author

@Jim8y Jim8y Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This should be in another PR and not here, if that is the case.

And for your notice, as I said in our meetings, this configuration has been tested in the past.

I think that you need more experimental tests before publishing things to quick and even writing new standards (that are not even necessary). I am not saying that the change is terrible, @Jim8y, but there are some consequences, I think that you did not analyse the entire scenario before pushing this and creating standards with it.

I will surely be against this change here in this PR for a series of reasons. Try this in a decentralized scenario or inserting delays (you can deal with that on docker) or create on AWS/AZURE in different locations.

We have setup a real testnet to carry out stress test and stability test for weeks. The network works fine and no issue. As you said should be in another pr thing, all changes in this pr now is for the service of and only serve the 3-seconds consensus, it is meaningless to add any of the prs and test any of the prs that only contain part of the changes in this pr. Casue non of the code in this pr means a thing without rest part.

We encourage small prs for the intention of easier review and test, its not like everything must be as small as possible.
If there is no review issue and no real logic problem, we should not split a complete and already tested pr.

Copy link
Member

Choose a reason for hiding this comment

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

Lots have changed in code since then. Would that affect anything like this? if so, we have to run these tests again to ensure its trueful.

You are right @cschuchardt88 , I was also saying that in my last message. Let's see

if (diff >= span)
span = TimeSpan.Zero;
else
Expand Down
2 changes: 1 addition & 1 deletion src/Plugins/StateService/Verification/VerificationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Neo.Plugins.StateService.Verification
{
class VerificationContext
{
private const uint MaxValidUntilBlockIncrement = 100;
private const uint MaxValidUntilBlockIncrement = 100; // Change to 500!??
private StateRoot root;
private ExtensiblePayload rootPayload;
private ExtensiblePayload votePayload;
Expand Down
2 changes: 1 addition & 1 deletion tests/Neo.Network.RPC.Tests/RpcTestCases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3148,7 +3148,7 @@
"protocol": {
"network": 0,
"validatorscount": 0,
"msperblock": 15000,
"msperblock": 3000,
"maxvaliduntilblockincrement": 1,
"maxtraceableblocks": 1,
"addressversion": 0,
Expand Down
2 changes: 1 addition & 1 deletion tests/Neo.Plugins.OracleService.Tests/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"ProtocolConfiguration": {
"Network": 5195086,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 512,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
Expand Down
10 changes: 5 additions & 5 deletions tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,23 +660,23 @@ public void TestReVerifyTopUnverifiedTransactionsIfNeeded()
Assert.AreEqual(0, _unit.VerifiedCount);
Assert.AreEqual(4, _unit.UnVerifiedCount);

AddTransactions(511); // Max per block currently is 512
Assert.AreEqual(511, _unit.VerifiedCount);
AddTransactions((int)(TestProtocolSettings.Default.MaxTransactionsPerBlock - 1)); // Max per block -1
Assert.AreEqual((int)(TestProtocolSettings.Default.MaxTransactionsPerBlock - 1), _unit.VerifiedCount);
Assert.AreEqual(4, _unit.UnVerifiedCount);

var result = _unit.ReVerifyTopUnverifiedTransactionsIfNeeded(1, GetSnapshot());
Assert.IsTrue(result);
Assert.AreEqual(512, _unit.VerifiedCount);
Assert.AreEqual((int)TestProtocolSettings.Default.MaxTransactionsPerBlock, _unit.VerifiedCount);
Assert.AreEqual(3, _unit.UnVerifiedCount);

result = _unit.ReVerifyTopUnverifiedTransactionsIfNeeded(2, GetSnapshot());
Assert.IsTrue(result);
Assert.AreEqual(514, _unit.VerifiedCount);
Assert.AreEqual((int)TestProtocolSettings.Default.MaxTransactionsPerBlock + 2, _unit.VerifiedCount);
Assert.AreEqual(1, _unit.UnVerifiedCount);

result = _unit.ReVerifyTopUnverifiedTransactionsIfNeeded(3, GetSnapshot());
Assert.IsFalse(result);
Assert.AreEqual(515, _unit.VerifiedCount);
Assert.AreEqual((int)TestProtocolSettings.Default.MaxTransactionsPerBlock + 3, _unit.VerifiedCount);
Assert.AreEqual(0, _unit.UnVerifiedCount);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/Neo.UnitTests/UT_ProtocolSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void TestGetMemoryPoolMaxTransactions()
[TestMethod]
public void TestGetMillisecondsPerBlock()
{
Assert.AreEqual((uint)15000, (uint)TestProtocolSettings.Default.MillisecondsPerBlock);
Assert.AreEqual((uint)3000, (uint)TestProtocolSettings.Default.MillisecondsPerBlock);
}

[TestMethod]
Expand Down Expand Up @@ -132,7 +132,7 @@ internal static string CreateHFSettings(string hf)
""ProtocolConfiguration"": {
""Network"": 860833102,
""AddressVersion"": 53,
""MillisecondsPerBlock"": 15000,
""MillisecondsPerBlock"": 3000,
""MaxTransactionsPerBlock"": 512,
""MemoryPoolMaxTransactions"": 50000,
""MaxTraceableBlocks"": 2102400,
Expand Down
2 changes: 1 addition & 1 deletion tests/Neo.UnitTests/test.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"ProtocolConfiguration": {
"Network": 860833102,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 512,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
Expand Down