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

Decrease block time to 3s #3612

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Decrease block time to 3s #3612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 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
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
4 changes: 2 additions & 2 deletions benchmarks/Neo.Benchmarks/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
"ProtocolConfiguration": {
"Network": 860833102,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MaxTransactionsPerBlock": 512,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 256,
Copy link
Contributor

Choose a reason for hiding this comment

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

MaxTransactionsPerBlock can be kept as is.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it need to be changed, I think that this comment here contradicts your comment below. I did not understand. You said to cut even more aggressively.

Copy link
Member

Choose a reason for hiding this comment

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

We need to reduce MaxTraceableBlocks, not MaxTransactionsPerBlock.

Copy link
Contributor

Choose a reason for hiding this comment

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

all can be reduced, each block wont be able to process that amount of transactions.

Copy link
Contributor

Choose a reason for hiding this comment

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

C# node can handle several thousands of simple NEP-17 transfers per second. It's all about MaxBlockSystemFee (#3552), not MaxTransactionsPerBlock.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that for a first release these proposed values are good.
They can be increased easily, the opposite is more complicated in my opnion.
So, I think we should just move forward.

"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
"Hardforks": {
Expand Down
4 changes: 2 additions & 2 deletions docs/config.json.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ 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`.
- **MaxTransactionsPerBlock**: Maximum number of transactions allowed per block. Default is `256`.

### MemoryPoolMaxTransactions
- **MemoryPoolMaxTransactions**: Maximum number of transactions that can be held in the memory pool. Default is `50000`.
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.CLI/config.fs.testnet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"ProtocolConfiguration": {
"Network": 91466898,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MaxTransactionsPerBlock": 512,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 256,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 17280,
"InitialGasDistribution": 5200000000000000,
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.CLI/config.testnet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"ProtocolConfiguration": {
"Network": 894710606,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MaxTransactionsPerBlock": 5000,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 256,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that it'd be ~73 days instead of current year. But I'd rather cut it down even more aggressively (see FS network settings).

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, let's say 64 or 128?

Copy link
Contributor

Choose a reason for hiding this comment

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

Opinions are welcome, your values remind me of E-letter chain. Nothing bad about that, but historic data (#2026) suggests our contracts are more used to somewhat longer history available. NeoFS networks use 17280 (3 days of 15s blocks).

"Hardforks": {
Expand Down
6 changes: 3 additions & 3 deletions src/Neo/ProtocolSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,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??
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is configurable per-network, so I'd rather leave the default as is and configure particular networks.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok


/// <summary>
/// Indicates the maximum number of transactions that can be contained in a block.
Expand Down Expand Up @@ -113,8 +113,8 @@ public record ProtocolSettings
StandbyCommittee = Array.Empty<ECPoint>(),
ValidatorsCount = 0,
SeedList = Array.Empty<string>(),
MillisecondsPerBlock = 15000,
MaxTransactionsPerBlock = 512,
MillisecondsPerBlock = 3000,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need to change defaults.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is better, @roman-khimov , because it may, in the future, reflect in some of our tests that use default. Better to keep consistent.

Copy link
Member

Choose a reason for hiding this comment

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

Because we will change testnet only before, it's better to avoid default changes

MaxTransactionsPerBlock = 256,
MemoryPoolMaxTransactions = 50_000,
MaxTraceableBlocks = 2_102_400,
InitialGasDistribution = 52_000_000_00000000,
Expand Down
3 changes: 3 additions & 0 deletions src/Neo/SmartContract/Native/NeoToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ internal override ContractTask InitializeAsync(ApplicationEngine engine, Hardfor
engine.SnapshotCache.Add(CreateStorageKey(Prefix_Committee), new StorageItem(cachedCommittee));
engine.SnapshotCache.Add(_votersCount, new StorageItem(System.Array.Empty<byte>()));
engine.SnapshotCache.Add(CreateStorageKey(Prefix_GasPerBlock).AddBigEndian(0u), new StorageItem(5 * GAS.Factor));
Copy link
Member Author

Choose a reason for hiding this comment

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

ping @shargon

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that maybe it should be enabled with 1 instead of 5 after certain height.
So the hardfork will be triggered by modification the SnapshotCache withnew StorageItem(5 * GAS.Factor);

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 moved to config.json?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can't change it without affecting post-genesis state. And I see zero reason to do this, mainnet/testnet don't care, this will be adjusted by the committee, private networks are fine with whatever default and they can also adjust it if needed.

// TODO - Add hardfork otherwise storage will be different
// After HARD FORK
engine.SnapshotCache.Add(_registerPrice, new StorageItem(1000 * GAS.Factor));

return Mint(engine, Contract.GetBFTAddress(engine.ProtocolSettings.StandbyValidators), TotalAmount, false);
}
return ContractTask.CompletedTask;
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 @@ -46,8 +46,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);
vncoelho marked this conversation as resolved.
Show resolved Hide resolved
// 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
12 changes: 6 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,8 @@ 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);
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 computed according the configured time per block


context.Block.Header.Timestamp = message.Timestamp;
context.Block.Header.Nonce = message.Nonce;
Expand Down Expand Up @@ -171,8 +171,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 +210,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
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!??
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoa. Unexpected one. It's totally different from the "regular" MVUBI, but 500 here would let more ExtensiblePayloads with signatures float around the network, I doubt the intention here was to have it related to time.

Copy link
Member

Choose a reason for hiding this comment

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

We should related it with the timeblock

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you guys should decide, that is why I left as comment.

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
4 changes: 2 additions & 2 deletions 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,8 +30,8 @@
"ProtocolConfiguration": {
"Network": 5195086,
"AddressVersion": 53,
"MillisecondsPerBlock": 15000,
"MaxTransactionsPerBlock": 512,
"MillisecondsPerBlock": 3000,
"MaxTransactionsPerBlock": 256,
"MemoryPoolMaxTransactions": 50000,
"MaxTraceableBlocks": 2102400,
"Hardforks": {
Expand Down
2 changes: 1 addition & 1 deletion tests/Neo.UnitTests/SmartContract/Native/UT_NeoToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ public void TestEconomicParameter()

(BigInteger, bool) result = Check_GetGasPerBlock(clonedCache, persistingBlock);
result.Item2.Should().BeTrue();
result.Item1.Should().Be(5 * NativeContract.GAS.Factor);
result.Item1.Should().Be(5 * NativeContract.GAS.Factor); // Test should occur with all hardfork enable TODO

persistingBlock = new Block { Header = new Header { Index = 10 } };
(VM.Types.Boolean, bool) result1 = Check_SetGasPerBlock(clonedCache, 10 * NativeContract.GAS.Factor, persistingBlock);
Expand Down
6 changes: 3 additions & 3 deletions tests/Neo.UnitTests/UT_ProtocolSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void TestGetMemoryPoolMaxTransactions()
[TestMethod]
public void TestGetMillisecondsPerBlock()
{
TestProtocolSettings.Default.MillisecondsPerBlock.Should().Be(15000);
TestProtocolSettings.Default.MillisecondsPerBlock.Should().Be(3000);
}

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