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.

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

Lower MaxBlockSystemFee limit #3552

Open
roman-khimov opened this issue Oct 24, 2024 · 5 comments
Open

Lower MaxBlockSystemFee limit #3552

roman-khimov opened this issue Oct 24, 2024 · 5 comments
Labels
Discussion Initial issue state - proposed but not yet accepted
Milestone

Comments

@roman-khimov
Copy link
Contributor

Summary or problem description
Our primary resource limiter on the network is GAS, all transactions are paid for and the presumption is that computational resources required to process transactions are proportional to the GAS cost of its system fee. We know sometimes this condition is not met and this opens possibilities for lowering the cost of DoS attacks (#2521, #2528, #2530, #2532, #2723, nspcc-dev/neo-go#3490, some others as well), but if we're to suppose #3510 is implemented and GAS cost follows the computational effort required better we still can have a MaxBlockSystemFee amount of GAS spent for execution in a block and it's not a low number.

The default right now is 1500 which is 10K of transactions like https://dora.coz.io/transaction/neo3/mainnet/0xbcd90d15b904c495294e99b954695be14d137d7c6d4c6fa8c0c2956a60f47f7b or 27K of transactions like https://dora.coz.io/transaction/neo3/mainnet/0xbcd90d15b904c495294e99b954695be14d137d7c6d4c6fa8c0c2956a60f47f7b. Of course there is also MaxTransactionsPerBlock, but it's not relevant in the context of this problem, we can pack less of more resource-demanding transactions into a block, they will ultimately be able to burn 1500 GAS overall. Considering proposals for lowering execution fee factor and other cost-related parameters overall computational effort can be quite huge for this 1500 GAS and it can delay subsequent blocks while ideally we want to have maximum processing time approximately equal to block time.

MaxBlockSystemFee can be lowered (even though unfortunately it's not a part of the Policy contract, see #2300), but we can't have it lower than NEO's RegisterPrice which currently is 1000 GAS and we have no intent to lower that (becoming a candidate should cost something real, see #2234). 1000 GAS is still quite a lot and can significant time to process, my estimate is that we probably want MaxBlockSystemFee to be in the lower hundreds range (100-400). But if we're to set MaxBlockSystemFee that low we won't be able to register candidates which just can't happen.

Do you have any solution you want to propose?
We can change registration mechanism from registerCandidate invocation to NEP-17 transfer of registration price to the NEO contract address. This will require NEO to implement NEP-27 handler and accept some GAS with additional pubkey passed in data, it can then burn this GAS immediately since it's not intended to hold it. But as the result of it we'll have some "normal" system fee value for registration transactions and MaxBlockSystemFee could be set lower than RegisterPrice.

Notice that it also solves the problem of registration transaction fee estimation since most RPC nodes are configured to 10-20 MaxGasInvoke and it's just impossible to make a proper invokefunction call for the registerCandidate method. Any software producing registration transactions currently has to perform some tricks like https://github.com/nspcc-dev/neo-go/blob/b8a65d3c37cfa429b8a5e135422bb1fa78762056/pkg/rpcclient/neo/neo.go#L327 to create a transaction.

This is a breaking change if we're to remove registerCandidate (but we can keep it for private networks and compatibility) or lower MaxBlockSystemFee to something less than RegisterPrice, but it's a rarely used method, so I doubt it's a big problem.

Where in the software does this update applies to?

  • NEO native contract
  • Consensus
@roman-khimov roman-khimov added the Discussion Initial issue state - proposed but not yet accepted label Oct 24, 2024
@Jim8y
Copy link
Contributor

Jim8y commented Oct 24, 2024

This can be a sure thing, definately will do, but need a thourough bench to decide the exact value.

@Jim8y Jim8y added this to the v3.8.0 milestone Oct 24, 2024
@AnnaShaleva
Copy link
Member

Agree, MaxBlockSystemFee should be aligned with execution fee factor, especially given the fact that there are discussions on upcoming reducing of BaseExecFee on N3 networks. Making candidate registration via transfer will work properly.

@roman-khimov
Copy link
Contributor Author

roman-khimov commented Oct 24, 2024

As for the MaxBlockSystemFee itself (the post above is mostly about obstacles we need to solve to be able to lower it at all).

I'm sorry, but it looks like we don't need fancy things like #2528, #2530, #2532, #2723 to really test our VM. Let's just remember that all of its fee model is built around the concept of a "NOP price" which is the basic unit we compare everything else against (#1875). So to get an idea of what can we compute with what amount of GAS we need to execute some NOPs. Like this, for example:

ISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISEhISKc

100 NOPs, one tiny JMP. Pretty easy script, in fact. The problem is that for testnet C# node (oopsie) it took this amount of time:

real    7m1,167s
user    0m0,015s
sys     0m0,019s

with pretty standard 20 GAS available to RPC invocation. Keep in mind that testnet currently has an execution fee factor of one (hi, @steven1227), so for mainnet that'd be at least three times better, something like 2m20s. Notice that with the standard execution fee factor of 30 that'd be more like 14s, so we can say that the original fee system as it was introduced with Neo 3.0 standard settings was much better balanced than what we have now. Still, even that only allowed for 20 GAS of execution effectively.

NeoGo is somewhat more efficient (or maybe it's a better machine), it allowed me to run the same script for

real    2m0.884s
user    0m0.008s
sys     0m0.008s

with 100 GAS and mainnet settings (execution fee factor 3). So it'd be 24s for 20GAS of current mainnet. Not perfect though, it's still significantly more than 15s.

What can we say of this overall? We can't safely have more than ~10-20 GAS MaxBlockSystemFee with the current mainnet settings. Which in fact would almost solves #3548 and some other problems, just imagine no more than 10-20 GAS of transactions in a block. Easy-peasy to process (mostly).

My suggestion for now is to reevaluate both ExecFeeFactor and MaxBlockSystemFee after #3510. Keeping in mind the possibility of raising the ExecFeeFactor. I think that the benefits of #3510 would allow to have ~same or lower fees for regular transactions in the end (since in fact they don't do a lot of computations and our opcode prices tend to be higher than what real scripts use in terms of resources), but we could ensure network safety at the same time.

@shargon
Copy link
Member

shargon commented Oct 26, 2024

We should move these things to policyContract

@dusmart
Copy link

dusmart commented Oct 28, 2024

I notice that @roman-khimov have posted a new method for burning GAS when the fee should be collected. Actually, this is better even if we don't consider the MaxBlockSystemFee limit.

As NEO N3 do not return systemFee that have not been used even if a transaction fails, the previous ApplicationEngine.AddFee (aka. System.Runtime.BurnGas syscall) method could be an entry for specific attacks.

Take an example from NNS's register method. If someone want to attack userA without earning profit, he could listen the mempool for userA's NNS register transaction and send a high-priority transaction that register the NNS immediately. Then, the userA would loss the fee (could be 200 GAS for a good domain for 1 year).

https://github.com/neo-project/non-native-contracts/blob/8d72b92e5e5705d763232bcc24784ced0fb8fc87/src/NameService/NameService.cs#L209-L224

Therefore, I strongly suggest that contract developers (including the native contract developers) do not use the System.Runtime.BurnGas as the fee burning mechanism. Instead, use what @roman-khimov suggested above. Then the user would not pay for nothing.

For NEO core developers, we may directly mark the original BurnGas as deprecated or providing a new version of BurnGAS that would charge the GAS during TX executing instead of charging from the systemFee (as before, throw exception if it can not burn the GAS).

roman-khimov added a commit to nspcc-dev/neo-go that referenced this issue Nov 24, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

See neo-project/neo#3552.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit to roman-khimov/neo that referenced this issue Nov 24, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

Fixes neo-project#3552.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit that referenced this issue Nov 29, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

Fixes #3552.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit to roman-khimov/neo that referenced this issue Dec 12, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

Fixes neo-project#3552.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit to roman-khimov/neo that referenced this issue Dec 19, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

Fixes neo-project#3552.

Signed-off-by: Roman Khimov <[email protected]>
NGDAdmin pushed a commit that referenced this issue Dec 20, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

Fixes #3552.

Signed-off-by: Roman Khimov <[email protected]>
roman-khimov added a commit to nspcc-dev/neo-go that referenced this issue Dec 22, 2024
Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

See neo-project/neo#3552.

Signed-off-by: Roman Khimov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

No branches or pull requests

5 participants