Exec Factor with decimals

[shargon]

Description

• Moved changes from Exec Factor with decimals #4202
• Moved from neo-project branch to user branch
• Rebased to master

Please @neo-project/core review this PR, as we agree in Centre Point it's important for the community.
Was decided in Centre Point to have two decimals in ExecutionFactor this PR tries to address it

Type of change

• Optimization (the change is only an optimization)
• Style (the change is only a code style for better maintenance or standard purpose)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

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 will test this week

[Jim8y]

give me a sec, i need to carefully check it. I think there are some inconsistency issues... but need to confirm

[erikzhang]

Why not we add a SetExecFeeFactorV2?

[erikzhang]

In this case, if there is a v2 value, we will ignore the v1 value.

[shargon]

Seems simple if we don't have an extra method, just change the factor

[AnnaShaleva]

Right now both GasConsumed and GasLeft are rounded to the upper integer. But this behaviour violates GasConsumed+GasLeft=(_feeAmount / FeeFactor) condition (we know that _feeAmount is divisible by FeeFactor by definition).

For GasConsumed rounding to the upper integer is correct since GasConsumed is used for transaction's fee calculation and e.g. transaction's SystemFee should cover sub-Datoshi execution cost.

For GasLeft I'm not sure what's the correct behaviour. Right now GasLeft is used only by System.Runtime.GasLeft interop, so rounding it to the upper value may be misleading for the smart contract (since in fact there's less gas remaining in the VM than System.Runtime.GasLeft tells). From another hand, rounding GasLeft to the lower integer may lead to the situation when System.Runtime.GasLeft returns 0 whereas some instructions still may be executed if we have some 0.999 GAS left in fact.

[AnnaShaleva]

Here's the statistics of System.Runtime.GasLeft interop usage on Mainnet up to 8179793 height:

• there are 200136 calls of this interop up to the current height
• there are 10 contracts, some of them use this call quite extensively:
  • ec268e9c642b7d09d10fe658bcb1cc63c0895d4d - 140042 calls - FlamingoBroker contract
  • 0e312c70ce6ed18d5702c6c5794c493d9ef46dc9 - 36516 calls - generator contract by CoZ
  • 76a8f8a7a901b29a33013b469949f4b08db15756 - 12556 calls - puppet NEP-11 contract by CoZ
  • 904deb56fdd9a87b48d89e0cc0ac3415f9207840 - 6764 calls - item NEP-11 by CoZ
  • a6f144d552cc96103393b9ca4e34cc8fd384f4bf - 1592 calls - lizard NEP-11 by CoZ
  • 963e74226c0cffa8a2369116f298579f9721b09d - 1390 calls - otter NEP-11 by CoZ
  • acd847761d49ce4396171e09886c64308f89a89a - 658 calls - Vaulten contract, some on-chain puzzle game
  • 5728017130c213cbc369c738f470d66628e5acf2 - 440 calls - package NEP-11 contract by CoZ
  • cc638d55d99fc81295daccbaf722b84f179fb9c4 - 159 calls - GhostMarket exchange contract
  • d44f73a8e3218ff137f5c0c731f478624758b1ec - 19 calls - SmartFLOCKSMinter contract

Here's the full log with additional data on usage: link.

The most active contract is FlamingoBroker contract. Its source code is not public, but here's the usage scenario taken from the bytecode (this pattern is used multiple times in the contract):

System.Runtime.GasLeft usage in `FlamingoBroker` contract

31039    LDLOC0          
31040    SYSCALL         System.Runtime.GasLeft (1488d8ce)
31045    SUB             
31046    DUP             
31047    PUSHINT64       -9223372036854775808 (0000000000000080)
31056    JMPGE           31060 (4/04)
31058    JMP             31072 (14/0e)
31060    DUP             
31061    PUSHINT64       9223372036854775807 (ffffffffffffff7f)
31070    JMPLE           31120 (50/32)
31072    PUSHINT128      18446744073709551615 (ffffffffffffffff0000000000000000)
31089    AND             
31090    DUP             
31091    PUSHINT64       9223372036854775807 (ffffffffffffff7f)
31100    JMPLE           31120 (20/14)
31102    PUSHINT128      18446744073709551616 (00000000000000000100000000000000)
31119    SUB             
31120    STLOC           10 (0a)
31122    LDLOC           9 (09)
31124    LDLOC           10 (0a)
31126    SUB             
31127    STLOC           11 (0b)
31129    LDLOC           11 (0b)
31131    PUSH0           
31132    LE              
31133    JMPIFNOT        31137 (4/04)
31135    JMP             31175 (40/28)
31137    LDLOC           11 (0b)
31139    DUP             
31140    PUSHINT64       -9223372036854775808 (0000000000000080)
31149    PUSHINT128      9223372036854775808 (00000000000000800000000000000000)
31166    WITHIN          
31167    JMPIF           31170 (3/03)
31169    THROW           
31170    SYSCALL         System.Runtime.BurnGas (c35a8cbc)
31175    LDSFLD          75 (4b)
31177    CALL_L          1235 (-29942/0a8bffff)
31182    RET             

So it looks like the overall usage pattern is "Burn some GAS until a specific amount of free GAS remains" (although we may ask @adrian-fjellberg or @odd86 for details). So @shargon, given this fact and the fact that it's related to trading, we assume that it's more safe to round GasLeft to the lower integer instead of rounding to the upper integer. Because smart contract relays on fact that the specified amount of GAS is 100% available to spent it to some invocation.

The second contract is generator contract deployed by CoZ, it has a little bit different pattern of usage:

System.Runtime.GasLeft usage in `generator` contract

6738     SYSCALL        System.Runtime.GasLeft (1488d8ce)
6743     STLOC          9 (09)
6745     LDLOC1         
6746     LDLOC          9 (09)
6748     SUB            
6749     STLOC          10 (0a)
6751     LDARG1         
6752     CALL_L         2137 (-4615/f9edffff)
6757     LDLOC          10 (0a)
6759     SUB            
6760     STLOC          11 (0b)
6762     LDLOC          11 (0b)
6764     SYSCALL        System.Runtime.BurnGas (c35a8cbc)
6769     LDLOC0         
6770     RET           

And it looks like rounding GasLeft to the lower integer doesn't hurt this pattern of usage as far.

The third contract is puppet by CoZ, it also has a very similar pattern of usage which looks to me like: "burn the amount of GAS left after execution of some contract bytecode (with an adjustment)"

System.Runtime.GasLeft usage in `puppet` contract

894      INITSLOT       10 local, 2 arg
897      SYSCALL        System.Runtime.GasLeft (1488d8ce) 
902      STLOC0         

... // execution of some side contract code

1114     SYSCALL        System.Runtime.GasLeft (1488d8ce) 
1119     STLOC6         
1120     LDLOC0         
1121     LDLOC6         
1122     SUB             
1123     STLOC          7 (07)
1125     LDLOC1         
1126     CALL_L         3231 (2105/39080000)
1131     STLOC          8 (08)
1133     LDLOC          8 (08)
1135     LDLOC          7 (07)
1137     SUB            
1138     STLOC          9 (09)
1140     LDLOC          9 (09)
1142     SYSCALL        System.Runtime.BurnGas (c35a8cbc)

And for this pattern rounding GasLeft to the lower integer also doesn't hurt.

I didn't check other contracts, but if needed, then I may analyze the usage pattern for the rest of the contracts and attach the results. @shargon, @roman-khimov, please let me know your thoughts on this.

[vncoelho]

Master branch is working again properly, as well as most basic features in this PR.

I will review code now, but it looks good to me.

[shargon]

@roman-khimov What do you think about solve this issue increasing the GAS decimals from 8 to 12, it's an @erikzhang idea, and I think it's a good one

[roman-khimov]

It'd be a perfect idea for 2021, pre-3.0. But now there are two things:

• formally it's a grave violation of NEP-17, since decimals() "MUST always return the same value every time it is invoked" (https://github.com/neo-project/proposals/blob/master/nep-17.mediawiki#decimals)
• realistically everyone knows GAS has 8 decimals: wallets, explorers, exchanges, contracts, scripts, everyone knows it and assumes it'll never change, so if it is to change things will break for sure

The only way to go from 8 to 12 to me is to migrate to a different token, but it's not fun either.

[erikzhang]

What are your thoughts on making NEO 4 a new chain without a native token? Like NeoX, we could transfer tokens across chains, and vice versa. This would allow us to develop new features on NEO 4 without worrying about compatibility.

[roman-khimov]

To me N4 is a compatible evolution of N3. I doubt anyone wants yet another chain with associated interoperability overheads. In that sense a new native GAS token on the same chain is easier to handle. Also, this particular change is expected to be delivered in 3.9.

[erikzhang]

Anyway, I need a clean codebase for launching N4 isomorphic sidechains in the future. For example, we might want to make the new DEX an application chain (sidechain) to support high TPS without clogging the main chain. I think the master branch can forgo all N3 compatibility, while master-3 must retain all N3 compatibility.

[roman-khimov]

I don't see any contradiction between sidechains and N3 evolution. In fact, one of things we're doing right now for NeoFS is a separate metadata chain that is N3-based, but has a different set of native contracts. IMO full N3 compatibility makes the solution stronger in that it's compatible with all things that exist for N3 already. It also makes supporting the platform easier overall, maintenance cost for separate N3 and N4 branches can be higher when they share a substantial percentage of code. All of the N4 things can be introduced via a set of N3 hardforks.

[deanragnarok]

I think launching N4 as a new chain would be a very hard sell to the Neo community. There are optics to be considered in addition to the technical considerations. Many still attribute the loss of momentum in the ecosystem to the Legacy-to-N3 migration. Even if this is a different scenario, it would be difficult to shake the perception that it’s happening all over again.

On top of that, the bridging of assets between Neo N3 and Neo X hasn’t been widely embraced yet. I understand there’s currently limited motivation for users to bridge, but adding a third chain into that mix could put things firmly in the too-hard-basket for users.

Personally, I think we should be simplifying and consolidating our efforts behind fewer platforms. We’ve only just shut down the Legacy chain, and the idea of having to maintain both N3 and a new N4 chain along with X, it feels like we’d be spreading ourselves thin all over again, let alone have to explain how it all fits together to new users.

[vncoelho]

This is the first time we are considering this approach for N4. Initially, I viewed N4 as a major compatible release (from 3.x → 4.0).

From my perspective, an N4 chain that retains NEO and GAS as the core of its economic model is ideal. The blockchain industry has evolved significantly, and the introduction of sidechains and a native DEX is undoubtedly the right direction to follow.

Although maintaining compatibility with N3 code is technically possible, it comes at a high cost. A new, cleaner codebase would open broader opportunities for innovation and allow us to better align with current industry standards.

Assuming NEO and GAS remain at the heart of the ecosystem, I believe a modern and well-structured implementation will bring greater safety, efficiency, and long-term sustainability.
Moreover, we should focus on designing and delivering robust, high-level bridges and cross-chain operations — an area that was not fully achieved in the N2→N3 or N3→NeoX transitions.

[gsmachado]

Moreover, we should focus on designing and delivering robust, high-level bridges and cross-chain operations — an area that was not fully achieved in the N2→N3 or N3→NeoX transitions.

In some weeks, NeoN3 <> NeoX will have an Arbitrary Message Bridge solution. I have a strong belief that people will like it and will support many use cases. For example, it will allow Neo X contracts to call Oracles of NeoN3.

So, despite what you said @vncoelho, we are indeed covering N3 <> NeoX transitions.

People have to realize that bridging things is a technically challenging and complex topic, for various reasons. It takes a long time.

[gsmachado]

I know that this comment doesn't help much, but I just want people to think about it.

From my experience (and I can be wrong 😅), let me just project (out of the blue) the two options we have:

1. If we do a separate chain for N4 (rollup/L2 or sidechain), it will take 6 to 12 months of development until we have a first version on mainnet (with less risks).
2. If we develop N4 as a set of hardforks in N3, it will take 18 to 24 months for completion (with more risks).

No matter how many resources we throw at it.

There are no rights or wrongs in the direction we take. But let's think about it.

[lllwvlvwlll]

Per @roman-khimov's comment, I do not think its feasible to extend the decimals without a large BD push. Its much simpler for internal teams to make changes to support this, but much more complicated to discuss the topic with exchanges and other integrators that are outside the ecosystem and may not be using "Neo N3 informed" architectures to make this change simple.

If we need to run an external campaign (outside of the ecosystem projects) to update all of the integrators to enable a reduced fee factor, I do not think its worth prioritizing on N3 EVEN THOUGH there is a huge value proposition.

Before discussing the implementation approach, we should probably align on "what" the project is. A new L1 initiative could very easily kill this ecosystem if we aren't careful so we need to make sure everyone is aligned on the "what" and "why" of this initiative to make sure we execute as a cohesive team.