core: init native interops in the genesis block #894

AnnaShaleva · 2020-04-20T17:45:24Z

closes #836

pkg/core/util.go

roman-khimov · 2020-04-23T20:22:06Z

@fyrchik: please take a look, I'll rebase it afterwards.

codecov · 2020-04-23T20:23:51Z

Codecov Report

Merging #894 into master will increase coverage by 0.82%.
The diff coverage is 59.66%.

@@            Coverage Diff             @@
##           master     #894      +/-   ##
==========================================
+ Coverage   63.32%   64.15%   +0.82%     
==========================================
  Files         195      192       -3     
  Lines       16451    16198     -253     
==========================================
- Hits        10418    10392      -26     
+ Misses       5495     5275     -220     
+ Partials      538      531       -7

Impacted Files	Coverage Δ
pkg/config/protocol_config.go	`70.00% <ø> (+6.36%)`	⬆️
pkg/core/interop_neo.go	`60.08% <ø> (-0.08%)`	⬇️
pkg/core/interops.go	`100.00% <ø> (ø)`
pkg/core/state/account.go	`95.52% <ø> (-0.43%)`	⬇️
pkg/core/storage/store.go	`42.30% <ø> (ø)`
pkg/core/transaction/transaction.go	`82.72% <ø> (+1.56%)`	⬆️
pkg/core/transaction/type.go	`80.48% <ø> (-3.19%)`	⬇️
pkg/rpc/response/result/account_state.go	`87.50% <ø> (-0.74%)`	⬇️
pkg/core/native/interop.go	`37.50% <25.00%> (+37.50%)`	⬆️
pkg/core/native/native_neo.go	`36.33% <37.95%> (+7.45%)`	⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa8d4d2...e6f5cff. Read the comment docs.

roman-khimov · 2020-04-26T20:00:28Z

Updated and improved, it's important to merge this tomorrow, so have a look at it.

fyrchik · 2020-04-27T08:12:46Z

pkg/core/state/native_state.go

+	}
+	r := io.NewBinReaderFromBuf(b)
+	balance.DecodeBinary(r)
+


Why do you have an empty line here but not in Bytes() ?

pkg/core/dao/dao.go

fyrchik · 2020-04-27T08:26:43Z

pkg/core/native/native_neo.go

+	if err != nil {
+		return err
+	}
+	if err := n.ModifyAccountVotes(oldAcc, ic.DAO, (&big.Int{}).Not(&acc.Balance)); err != nil {


It is not Not, but Neg.
We also use new(big.Int) in every other place.

It is not Not, but Neg.

Yep, it was fixed in some subsequent commit, maybe I need to shuffle these changes a little.

fyrchik · 2020-04-27T08:42:21Z

pkg/consensus/consensus.go

 	var (
 		pKeys []*keys.PublicKey
 		err   error
 	)
-	if len(txx) == 0 {


When invoked with non-zero sized slice, it is used for calculating NextConsensus field.
DBFT needs to be changed, but for now we can return GetNextBlockValidators when invoked with non-zero slice.
https://github.com/neo-project/neo/blob/master/src/neo/Consensus/ConsensusContext.cs#L349

I don't think DBFT needs any change wrt this. We already do return next block validators from GetValidators.

fyrchik · 2020-04-27T08:47:34Z

pkg/core/native/native_neo.go

@@ -446,7 +450,7 @@ func (n *NEO) getNextBlockValidators(ic *interop.Context, _ []vm.StackItem) vm.S
 func (n *NEO) GetNextBlockValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) {
 	si := d.GetStorageItem(n.Hash, nextValidatorsKey)
 	if si == nil {
-		return bc.GetStandByValidators()


If validatorsCount is not yet initialized, GetValidatorsInternal can fail, no?
https://github.com/neo-project/neo/blob/master/src/neo/SmartContract/Native/Tokens/NeoToken.cs#L264

It's initialized now in Initialize at block 0, so I don't think we have any problem.

closes #836

The notion of NativeContractState shouldn't ever existed, native contract is a contract and its state is saved as regular contract state which is critical because we'll have MPT calculations over this state soon. Initial minting should be done in Neo.Native.Deploy because it generates notification that should have proper transaction context. RegisterNative() shouldn't exist as a public method, native contracts are only registered at block 0 and they can do it internally, no outside user should be able to mess with it. Move some structures from `native` package to `interop` also to avoid circular references as interop.Context has to have a list of native contracts (exposing them via Blockchainer is again too dangerous, it's too powerful tool).

It fails at the moment and it doesn't make sense at conceptual level.

nil is not a good StackItem, we have proper VM-level Null for this.

As it should be done (although current serialization format is not quite right).

This technically breaks voting with UTXO-based NEO (processTXWithValidators*), but we're moving towards the new system.

As it's one of the use cases.

It has all the methods required now, so you can register, vote and get voting results. Fixes #865.

They're completely replaced now by the NEO native contract voting system.

roman-khimov · 2020-04-27T13:09:30Z

Updated to mention #865 as it technically fixes it now.

AnnaShaleva added the neo3 label Apr 20, 2020

AnnaShaleva requested a review from roman-khimov April 20, 2020 17:45

AnnaShaleva self-assigned this Apr 20, 2020

fyrchik reviewed Apr 21, 2020

View reviewed changes

pkg/core/util.go Outdated Show resolved Hide resolved

AnnaShaleva force-pushed the neo3/genesis/init_native_interops branch 2 times, most recently from 1455f62 to 07b9cd8 Compare April 21, 2020 10:00

roman-khimov force-pushed the neo3/genesis/init_native_interops branch from 07b9cd8 to 0642931 Compare April 23, 2020 20:18

roman-khimov marked this pull request as ready for review April 23, 2020 20:21

roman-khimov requested a review from fyrchik April 23, 2020 20:21

roman-khimov changed the title ~~core: init native interops into the genesis block~~ core: init native interops in the genesis block Apr 23, 2020

roman-khimov force-pushed the neo3/genesis/init_native_interops branch from 0642931 to 4107f06 Compare April 24, 2020 09:01

roman-khimov mentioned this pull request Apr 26, 2020

Eliminate stored value serialization differences in native contracts #908

Closed

fyrchik suggested changes Apr 27, 2020

View reviewed changes

roman-khimov mentioned this pull request Apr 27, 2020

Make search results deterministic and following C# implementation #909

Closed

AnnaShaleva and others added 11 commits April 27, 2020 12:30

core: init native interops in the genesis block

869c7d6

closes #836

native: don't distribute gas at block 0

1dcace1

It fails at the moment and it doesn't make sense at conceptual level.

native: put proper Null StackItem for transfer event

8c02c6b

nil is not a good StackItem, we have proper VM-level Null for this.

native: store NEO and GAS state in the storage

4e8ee69

As it should be done (although current serialization format is not quite right).

core/native: store validators in NEO native contract state

3476a18

This technically breaks voting with UTXO-based NEO (processTXWithValidators*), but we're moving towards the new system.

core/native: move ValidatorsCount processing into native NEO contract

0646367

native: distribute GAS even for empty transfers

36c6c66

As it's one of the use cases.

native: update voting information when changing Neo balance

66c80d4

core/native: move NextValidators storage to NEO contract

bc4a6a6

core/native: move Votes from account to native NEO state

2fa3bdf

roman-khimov force-pushed the neo3/genesis/init_native_interops branch from 9016cb4 to 5c9e3e5 Compare April 27, 2020 09:31

roman-khimov requested a review from fyrchik April 27, 2020 09:49

roman-khimov added 2 commits April 27, 2020 16:06

core: switch to the new voting system (via native NEO contract)

b83e84c

It has all the methods required now, so you can register, vote and get voting results. Fixes #865.

transaction: drop Enrollment and State types

e6f5cff

They're completely replaced now by the NEO native contract voting system.

roman-khimov force-pushed the neo3/genesis/init_native_interops branch from 5c9e3e5 to e6f5cff Compare April 27, 2020 13:07

fyrchik approved these changes Apr 27, 2020

View reviewed changes

roman-khimov merged commit de91418 into master Apr 27, 2020

roman-khimov deleted the neo3/genesis/init_native_interops branch April 27, 2020 13:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: init native interops in the genesis block #894

core: init native interops in the genesis block #894

AnnaShaleva commented Apr 20, 2020

roman-khimov commented Apr 23, 2020

codecov bot commented Apr 23, 2020 •

edited

Loading

roman-khimov commented Apr 26, 2020

fyrchik Apr 27, 2020

fyrchik Apr 27, 2020

roman-khimov Apr 27, 2020

fyrchik Apr 27, 2020

roman-khimov Apr 27, 2020

fyrchik Apr 27, 2020

roman-khimov Apr 27, 2020

roman-khimov commented Apr 27, 2020

core: init native interops in the genesis block #894

core: init native interops in the genesis block #894

Conversation

AnnaShaleva commented Apr 20, 2020

roman-khimov commented Apr 23, 2020

codecov bot commented Apr 23, 2020 • edited Loading

Codecov Report

roman-khimov commented Apr 26, 2020

fyrchik Apr 27, 2020

Choose a reason for hiding this comment

fyrchik Apr 27, 2020

Choose a reason for hiding this comment

roman-khimov Apr 27, 2020

Choose a reason for hiding this comment

fyrchik Apr 27, 2020

Choose a reason for hiding this comment

roman-khimov Apr 27, 2020

Choose a reason for hiding this comment

fyrchik Apr 27, 2020

Choose a reason for hiding this comment

roman-khimov Apr 27, 2020

Choose a reason for hiding this comment

roman-khimov commented Apr 27, 2020

codecov bot commented Apr 23, 2020 •

edited

Loading