Remove some settings from policy contract

[erikzhang]

No description provided.

[roman-khimov]

A step back to Neo 2 IMO. I'd rather have protocol configuration file contain the bare minimum to successfully operate on the network/process chain (like magic and seed nodes list) and everything else should be discoverable from on-chain data. Consider updating these values, going through chain is like sending one transaction and everything works, but changing configuration files requires synchronization between CN maintainers and restarts.

[erikzhang]

These settings are more about consensus. And the consensus is moved to plugin. So we should remove these settings from core.

[shargon]

I think that any consensus need these kind of limits, so it's good to have this configuration shared and stored in the chain

[erikzhang]

But who can modify these settings? The committee? I don't think so. Maybe the core developers should be responsible for modifying them.

[roman-khimov]

Committee runs the network, so committee sets the setting. In a transparent and traceable way.

[Tommo-L]

The code is good for me. These parameters are mainly protocol security parameters, which can not be determined by committee.

[erikzhang]

Go?

[cloud8little]

Go?

Testing. Please wait.

[shargon]

How the core developers will ensure that all the nodes will have the same policiy file? maybe we should add this configuration inside the blockchain and allow only core developers to change it

[erikzhang]

maybe we should add this configuration inside the blockchain and allow only core developers to change it

I think it is impossible.

[shargon]

I think it is impossible.

With hardcoded url and oracle we can update a json in mainnet-config.neo.org (and redirect this dns to github url).

[erikzhang]

I don't think a decentralized blockchain should rely on a centralized website.

[shargon]

I don't think a decentralized blockchain should rely on a centralized website.

It will be the same with the config file.

[erikzhang]

It will be the same with the config file.

It won't. With config file you can "choose" to update or not.

[roman-khimov]

But you can't set arbitrary values there, the network won't function properly if there is any disagreement between CNs wrt this values.

[Qiao-Jin]

If this check is removed, how to stop such transactions from getting into Mempool?

[erikzhang]

We should treat service, storage and CPU fees differently, otherwise the restrictions on fees are meaningless. This can be done later.

[cloud8little]

It seems there is a problem here, when CN all update their protocol settings to set a smaller MaxTransactionsPerBlock, and before the change, there is already a bigger MaxTransactionsPerBlock number of TX persisted in the previous blocks. then the CN failed to start.

scenario:
CN starts with initial MaxTransactionsPerBlock=200, and send massive tx, some blocks contain the maximum 200 blocks. then stop the CN nodes, and update MaxTransactionsPerBlock to 100, fail to start.

        public void Deserialize(BinaryReader reader)
        {
            Header = reader.ReadSerializable<Header>();
            Transactions = reader.ReadSerializableArray<Transaction>((int)ProtocolSettings.Default.MaxTransactionsPerBlock);
            if (Transactions.Distinct().Count() != Transactions.Length)
                throw new FormatException();
            if (MerkleTree.ComputeRoot(Transactions.Select(p => p.Hash).ToArray()) != Header.MerkleRoot)

[erikzhang]

when CN all update their protocol settings to set a smaller MaxTransactionsPerBlock, and before the change, there is already a bigger MaxTransactionsPerBlock number of TX persisted in the previous blocks. then the CN failed to start.

This field cannot be set smaller.

[erikzhang]

Go?

[cloud8little]

Go?

I am good. Except a little concern that there is a possibility to set the max transaction smaller than the previous configuration

[erikzhang]

there is a possibility to set the max transaction smaller than the previous configuration

This is impossible. Because it is hard-coded, and we will only make it larger.

[bettybao1209]

Should we eliminate the use of ProtocolSettings.Default here?

[erikzhang]

It should be fixed by #2337