Fix sys_fee calculation in block persist

[ixje]

What current issue(s) does this address, or what feature is it adding?
Fix #902

#902 is the unforeseen result of fixing (among others) a wrong net_fee calculation reported in #749.
It all started when I there changed the SystemFee implementation of the InvocationTransactionfrom

   return self.Gas // Fixed8.FD()

to

   return self.Gas

to be inline with neocore's (ref)

public override Fixed8 SystemFee => Gas;

This together with another change fixed #749. What I unfortunately did not foresee is that it would cause the issue described in #902, which is caused by this line:

neo-python/neo/Implementations/Blockchains/LevelDB/LevelDBBlockchain.py

Line 666 in fe90f62

amount_sysfee = self.GetSysFeeAmount(block.PrevHash) + block.TotalFees().value

According to the neocore implementation the equivalent of our block.TotalFees().value would be (ref)

(long)block.Transactions.Sum(p => p.SystemFee)

The crux of the problem is the cast from Fixed8 to long, which is implemented as (ref)

        public static explicit operator long(Fixed8 value)
        {
            return value.value / D;
        }

Our version did not apply this cast (essentially divide by Fixed8.FD()) on the TotalFees. I believe this is just an error that originated in the faulty InvocationTransaction.SystemFee implementation and that propagated throughout the code base.

How did you solve this problem?
First off, I applied the same division to the TotalFees end result.

Second, since we now get a float instead of an int, I changed the logic for creating a 8 bytes bytearray from an int, to use struct.unpack("<d", float) (which creates an 8 bytes double in little endian format). This appears to be inline with storing a float (8 bytes in C#).

Third, I took the time to cross referenced the implementation of all other transactions on system fee and network fee with the C# code and made 2 changes there.

Finally, I updated all tests + fixtures (because the db storage is different)

How did you make sure your solution works?
make test + compare with neo-cli

Are there any special changes in the code that we should be aware of?
yes, we change the storage format of the system fee in the db. Previously

DBPrefix.DATA_Block + block hash + INT systemfee bytes (8) + block

now

DBPrefix.DATA_Block + block hash + FLOAT systemfee bytes (8) + block

This will require a resync of the chain 😞 , but should at least fix all known net_fee and sys_fee discrepancies.

Please check the following, if applicable:

• Did you add any tests?
• Did you run make lint?
• Did you run make test?
• Are you making a PR to a feature branch or development rather than master?
• Did you add an entry to CHANGELOG.rst? (if not, please do)

[coveralls]

Coverage increased (+0.002%) to 86.892% when pulling 6f0d0a4 on ixje:sys_fee into 0664f8a on CityOfZion:development.

[jseagrave21]

Hey there, I noticed this also requires an update of the fixtures. Any chance you were able to implement v9 from #716 and then update to v10? Maybe merging this fix will also allow #716 to be merged?

[ixje]

going to merge even though mainnet/testnet fixtures still can't be made (VM just got updated again)