Remove MillisecondsPerBlock, MaxValidUntilBlockIncrement, MaxTraceableBlocks from PolicyContract

[erikzhang]

…eBlocks from PolicyContract

Remove MillisecondsPerBlock, MaxValidUntilBlockIncrement, MaxTraceableBlocks from PolicyContract.

1. The block time will be dynamically determined by the consensus nodes based on network load.
2. MaxValidUntilBlockIncrement and MaxTraceableBlocks should be exposed as immutable values.

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

• Unit Testing
• Run Application
• Local Computer Tests
• No Testing

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[vncoelho]

I understand the change and looks fine for its purpose.

The reasoning here is probably to avoid unnecessary state changes on a SC and the parameter become a stateless decision agreed by CN's.

On the hand other, one may need to monitor P2P to understand network heartbeat, right?

[shargon]

The block time will be dynamically determined by the consensus nodes based on network load.

But this must be configured by the policy, isn't it? so instead of remove we should move this config to each level

[erikzhang]

But this must be configured by the policy, isn't it? so instead of remove we should move this config to each level

I hope this parameter will be adaptive in the future, requiring no adjustment. For now, we can treat it as a configuration item for the consensus plugin.

[vncoelho]

As I asked above, what is the main reason behind this? To avoid state changes on the blockchain, right?

[shargon]

How can we ensure the deterministic behavior between nodes if any node can have a different configuration? We can use a hash of the config for Blockchain id, but seems easier in policy

[vncoelho]

In terms of behavior, as it will be we will only measure due to performance.
Maybe with something like neo-project/proposals#211 (comment)

On the other hand, I am trying to understand the real motivation behind this. If this is to avoid state changes.

In particular, I believe that for such things we should implement a temporary state, such as Blobs.

[vncoelho]

So, instead of writing on a Native Contract we publish that states with Blobs that will become unavailable in the future.

That is what ETH did for making Layer 2 cheaper.

[erikzhang]

How can we ensure the deterministic behavior between nodes if any node can have a different configuration? We can use a hash of the config for Blockchain id, but seems easier in policy

There's no difference between putting parameters in a configuration file and hard-coding them in the code. The purpose of a configuration file is to provide different parameters for different networks, not to allow different nodes to modify them arbitrarily.

[Wi1l-B0t]

Remove MillisecondsPerBlock after dynamic block time is available?

[erikzhang]

Remove MillisecondsPerBlock after dynamic block time is available?

In fact, another important reason for removing MillisecondsPerBlock from the PolicyContract is that its current implementation is incorrect. Changing the block time necessitates changing the GAS distribution model. Currently, GAS is distributed based on block height, and changing the block time alters the rate of GAS issuance. Under the current GAS distribution model, the block time must remain fixed.

[shargon]

Let's check the expected behavior when finish the dynamic block interval