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

*: move Notary contract out of P2PSigExtensions under Echidna hardfork #3478

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft
3 changes: 2 additions & 1 deletion cli/nep_test/nep17_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"github.com/nspcc-dev/neo-go/internal/testcli"
"github.com/nspcc-dev/neo-go/pkg/core/native/nativehashes"
"github.com/nspcc-dev/neo-go/pkg/core/native/nativenames"
"github.com/nspcc-dev/neo-go/pkg/encoding/address"
"github.com/nspcc-dev/neo-go/pkg/encoding/fixedn"
Expand Down Expand Up @@ -236,7 +237,7 @@ func TestNEP17Transfer(t *testing.T) {
e.CheckAwaitableTxPersisted(t)
})

cmd = append(cmd, "--to", address.Uint160ToString(e.Chain.GetNotaryContractScriptHash()),
cmd = append(cmd, "--to", address.Uint160ToString(nativehashes.Notary),
"[", testcli.ValidatorAddr, strconv.Itoa(int(validTil)), "]")

t.Run("with data", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion docs/node-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ protocol-related settings described in the table below.
| MaxValidUntilBlockIncrement | `uint32` | `5760` | Upper height increment limit for transaction's ValidUntilBlock field value relative to the current blockchain height, exceeding which a transaction will fail validation. It is set to estimated daily number of blocks with 15s interval by default. |
| MemPoolSize | `int` | `50000` | Size of the node's memory pool where transactions are stored before they are added to block. |
| P2PNotaryRequestPayloadPoolSize | `int` | `1000` | Size of the node's P2P Notary request payloads memory pool where P2P Notary requests are stored before main or fallback transaction is completed and added to the chain.<br>This option is valid only if `P2PSigExtensions` are enabled. | Not supported by the C# node, thus may affect heterogeneous networks functionality. |
| P2PSigExtensions | `bool` | `false` | Enables following additional Notary service related logic:<br>• Transaction attribute `NotaryAssisted`<br>• Network payload of the `P2PNotaryRequest` type<br>• Native `Notary` contract<br>• Notary node module | Not supported by the C# node, thus may affect heterogeneous networks functionality. |
| P2PSigExtensions | `bool` | `false` | Enables following additional Notary service related logic:<br>• Network payload of the `P2PNotaryRequest` type<br>• Notary node module | Not supported by the C# node, thus may affect heterogeneous networks functionality. |
| P2PStateExchangeExtensions | `bool` | `false` | Enables the following P2P MPT state data exchange logic: <br>• `StateSyncInterval` protocol setting <br>• P2P commands `GetMPTDataCMD` and `MPTDataCMD` | Not supported by the C# node, thus may affect heterogeneous networks functionality. Can be supported either on MPT-complete node (`KeepOnlyLatestState`=`false`) or on light GC-enabled node (`RemoveUntraceableBlocks=true`) in which case `KeepOnlyLatestState` setting doesn't change the behavior, an appropriate set of MPTs is always stored (see `RemoveUntraceableBlocks`). |
| ReservedAttributes | `bool` | `false` | Allows to have reserved attributes range for experimental or private purposes. |
| SeedList | `[]string` | [] | List of initial nodes addresses used to establish connectivity. |
Expand Down
7 changes: 7 additions & 0 deletions internal/basicchain/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path/filepath"
"testing"

"github.com/nspcc-dev/neo-go/pkg/config"
"github.com/nspcc-dev/neo-go/pkg/core/native"
"github.com/nspcc-dev/neo-go/pkg/core/native/nativenames"
"github.com/nspcc-dev/neo-go/pkg/core/native/noderoles"
Expand Down Expand Up @@ -81,6 +82,11 @@ func Init(t *testing.T, rootpath string, e *neotest.Executor) {
t.Fatal("P2PSigExtensions should be enabled to init basic chain")
}

const notaryDepositHeight uint32 = 8
echidnaH, ok := e.Chain.GetConfig().Hardforks[config.HFEchidna.String()]
require.Truef(t, ok, "%s hardfork should be enabled since basic chain uses Notary contract", config.HFEchidna.String())
require.LessOrEqualf(t, echidnaH, notaryDepositHeight-1, "%s hardfork should be enabled starting from height %d, got: %d", config.HFEchidna.String(), notaryDepositHeight-1, echidnaH)

var (
// examplesPrefix is a prefix of the example smart-contracts.
examplesPrefix = filepath.Join(rootpath, "examples")
Expand Down Expand Up @@ -189,6 +195,7 @@ func Init(t *testing.T, rootpath string, e *neotest.Executor) {
// Block #8: deposit some GAS to notary contract for priv0.
transferTxH = gasPriv0Invoker.Invoke(t, true, "transfer", priv0ScriptHash, notaryHash, 10_0000_0000, []any{priv0ScriptHash, int64(e.Chain.BlockHeight() + 1000)})
t.Logf("notaryDepositTxPriv0: %v", transferTxH.StringLE())
require.Equal(t, uint32(notaryDepositHeight), e.Chain.BlockHeight(), "notaryDepositHeight constant is out of date")

// Block #9: designate new Notary node.
ntr, err := wallet.NewWalletFromFile(path.Join(notaryModulePath, "./testdata/notary1.json"))
Expand Down
33 changes: 12 additions & 21 deletions internal/fakechain/fakechain.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,18 @@ import (
type FakeChain struct {
config.Blockchain
*mempool.Pool
blocksCh []chan *block.Block
Blockheight atomic.Uint32
PoolTxF func(*transaction.Transaction) error
poolTxWithData func(*transaction.Transaction, any, *mempool.Pool) error
blocks map[util.Uint256]*block.Block
hdrHashes map[uint32]util.Uint256
txs map[util.Uint256]*transaction.Transaction
VerifyWitnessF func() (int64, error)
MaxVerificationGAS int64
NotaryContractScriptHash util.Uint160
NotaryDepositExpiration uint32
PostBlock []func(func(*transaction.Transaction, *mempool.Pool, bool) bool, *mempool.Pool, *block.Block)
UtilityTokenBalance *big.Int
blocksCh []chan *block.Block
Blockheight atomic.Uint32
PoolTxF func(*transaction.Transaction) error
poolTxWithData func(*transaction.Transaction, any, *mempool.Pool) error
blocks map[util.Uint256]*block.Block
hdrHashes map[uint32]util.Uint256
txs map[util.Uint256]*transaction.Transaction
VerifyWitnessF func() (int64, error)
MaxVerificationGAS int64
NotaryDepositExpiration uint32
PostBlock []func(func(*transaction.Transaction, *mempool.Pool, bool) bool, *mempool.Pool, *block.Block)
UtilityTokenBalance *big.Int
}

// FakeStateSync implements the StateSync interface.
Expand Down Expand Up @@ -111,14 +110,6 @@ func (chain *FakeChain) GetNotaryDepositExpiration(acc util.Uint160) uint32 {
panic("TODO")
}

// GetNotaryContractScriptHash implements the Blockchainer interface.
func (chain *FakeChain) GetNotaryContractScriptHash() util.Uint160 {
if !chain.NotaryContractScriptHash.Equals(util.Uint160{}) {
return chain.NotaryContractScriptHash
}
panic("TODO")
}

// GetNotaryBalance implements the Blockchainer interface.
func (chain *FakeChain) GetNotaryBalance(acc util.Uint160) *big.Int {
panic("TODO")
Expand Down
3 changes: 3 additions & 0 deletions pkg/config/hardfork.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const (
// different ApplicationLogs comparing to the C# node, but the node states
// match. See #3485 for details.
HFDomovoi // Domovoi
// HFEchidna represents hard-fork introduced in #3476 (ported from
// https://github.com/neo-project/neo/pull/3175).
HFEchidna // Echidna
// hfLast denotes the end of hardforks enum. Consider adding new hardforks
// before hfLast.
hfLast
Expand Down
8 changes: 6 additions & 2 deletions pkg/config/hardfork_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 17 additions & 15 deletions pkg/core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"github.com/nspcc-dev/neo-go/pkg/core/mempool"
"github.com/nspcc-dev/neo-go/pkg/core/mpt"
"github.com/nspcc-dev/neo-go/pkg/core/native"
"github.com/nspcc-dev/neo-go/pkg/core/native/nativehashes"
"github.com/nspcc-dev/neo-go/pkg/core/native/noderoles"
"github.com/nspcc-dev/neo-go/pkg/core/state"
"github.com/nspcc-dev/neo-go/pkg/core/stateroot"
Expand Down Expand Up @@ -1070,7 +1071,7 @@
hfs := bc.config.Hardforks
if hf != nil {
start, ok := hfs[hf.String()]
if !ok || start < blockHeight {
if !ok || start > blockHeight {
return false
}
}
Expand Down Expand Up @@ -2104,26 +2105,30 @@
}

// GetNotaryBalance returns Notary deposit amount for the specified account.
// Default value is returned if Notary contract is not yet active.
func (bc *Blockchain) GetNotaryBalance(acc util.Uint160) *big.Int {
if !bc.isHardforkEnabled(bc.contracts.Notary.ActiveIn(), bc.BlockHeight()) {
return nil

Check warning on line 2111 in pkg/core/blockchain.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/blockchain.go#L2111

Added line #L2111 was not covered by tests
}
return bc.contracts.Notary.BalanceOf(bc.dao, acc)
}

// GetNotaryServiceFeePerKey returns a NotaryAssisted transaction attribute fee
// per key which is a reward per notary request key for designated notary nodes.
// Default value is returned if Notary contract is not yet active.
func (bc *Blockchain) GetNotaryServiceFeePerKey() int64 {
return bc.contracts.Policy.GetAttributeFeeInternal(bc.dao, transaction.NotaryAssistedT)
}

// GetNotaryContractScriptHash returns Notary native contract hash.
func (bc *Blockchain) GetNotaryContractScriptHash() util.Uint160 {
if bc.P2PSigExtensionsEnabled() {
return bc.contracts.Notary.Hash
if !bc.isHardforkEnabled(&transaction.NotaryAssistedActivation, bc.BlockHeight()) {
return 0

Check warning on line 2121 in pkg/core/blockchain.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/blockchain.go#L2121

Added line #L2121 was not covered by tests
}
return util.Uint160{}
return bc.contracts.Policy.GetAttributeFeeInternal(bc.dao, transaction.NotaryAssistedT)
}

// GetNotaryDepositExpiration returns Notary deposit expiration height for the specified account.
// Default value is returned if Notary contract is not yet active.
func (bc *Blockchain) GetNotaryDepositExpiration(acc util.Uint160) uint32 {
if !bc.isHardforkEnabled(bc.contracts.Notary.ActiveIn(), bc.BlockHeight()) {
return 0

Check warning on line 2130 in pkg/core/blockchain.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/blockchain.go#L2130

Added line #L2130 was not covered by tests
}
return bc.contracts.Notary.ExpirationOf(bc.dao, acc)
}

Expand Down Expand Up @@ -2695,10 +2700,10 @@
return fmt.Errorf("%w: conflicting transaction %s is already on chain", ErrInvalidAttribute, conflicts.Hash.StringLE())
}
case transaction.NotaryAssistedT:
if !bc.config.P2PSigExtensions {
return fmt.Errorf("%w: NotaryAssisted attribute was found, but P2PSigExtensions are disabled", ErrInvalidAttribute)
if !bc.isHardforkEnabled(&transaction.NotaryAssistedActivation, bc.BlockHeight()) {
return fmt.Errorf("%w: NotaryAssisted attribute was found, but %s is not active yet", ErrInvalidAttribute, transaction.NotaryAssistedActivation)
}
if !tx.HasSigner(bc.contracts.Notary.Hash) {
if !tx.HasSigner(nativehashes.Notary) {
return fmt.Errorf("%w: NotaryAssisted attribute was found, but transaction is not signed by the Notary native contract", ErrInvalidAttribute)
}
default:
Expand Down Expand Up @@ -3088,9 +3093,6 @@

// GetMaxNotValidBeforeDelta returns maximum NotValidBeforeDelta Notary limit.
func (bc *Blockchain) GetMaxNotValidBeforeDelta() (uint32, error) {
if !bc.config.P2PSigExtensions {
panic("disallowed call to Notary") // critical error, thus panic.
}
if !bc.isHardforkEnabled(bc.contracts.Notary.ActiveIn(), bc.BlockHeight()) {
return 0, fmt.Errorf("native Notary is active starting from %s", bc.contracts.Notary.ActiveIn().String())
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/core/blockchain_core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ func TestNewBlockchain_InitHardforks(t *testing.T) {
config.HFBasilisk.String(): 0,
config.HFCockatrice.String(): 0,
config.HFDomovoi.String(): 0,
config.HFEchidna.String(): 0,
}, bc.GetConfig().Hardforks)
})
t.Run("empty set", func(t *testing.T) {
Expand Down Expand Up @@ -401,14 +402,15 @@ func TestNewBlockchain_InitHardforks(t *testing.T) {
})
t.Run("all present", func(t *testing.T) {
bc := newTestChainWithCustomCfg(t, func(c *config.Config) {
c.ProtocolConfiguration.Hardforks = map[string]uint32{config.HFAspidochelone.String(): 5, config.HFBasilisk.String(): 10, config.HFCockatrice.String(): 15, config.HFDomovoi.String(): 20}
c.ProtocolConfiguration.Hardforks = map[string]uint32{config.HFAspidochelone.String(): 5, config.HFBasilisk.String(): 10, config.HFCockatrice.String(): 15, config.HFDomovoi.String(): 20, config.HFEchidna.String(): 25}
require.NoError(t, c.ProtocolConfiguration.Validate())
})
require.Equal(t, map[string]uint32{
config.HFAspidochelone.String(): 5,
config.HFBasilisk.String(): 10,
config.HFCockatrice.String(): 15,
config.HFDomovoi.String(): 20,
config.HFEchidna.String(): 25,
}, bc.GetConfig().Hardforks)
})
}
13 changes: 9 additions & 4 deletions pkg/core/blockchain_neotest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) {

_, _, _, err = chain.NewMultiWithCustomConfigAndStoreNoCheck(t, customConfig, cache)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), fmt.Sprintf("native %s: version mismatch for the latest hardfork Domovoi (stored contract state differs from autogenerated one)", nativenames.CryptoLib)), err)
require.True(t, strings.Contains(err.Error(), fmt.Sprintf("native %s: version mismatch for the latest hardfork Echidna (stored contract state differs from autogenerated one)", nativenames.CryptoLib)), err)
})

t.Run("good", func(t *testing.T) {
Expand Down Expand Up @@ -2057,7 +2057,12 @@ func TestBlockchain_VerifyTx(t *testing.T) {
}
t.Run("Disabled", func(t *testing.T) {
bcBad, validatorBad, committeeBad := chain.NewMultiWithCustomConfig(t, func(c *config.Blockchain) {
c.P2PSigExtensions = false
c.P2PSigExtensions = true
c.Hardforks = map[string]uint32{
config.HFAspidochelone.String(): 0,
config.HFBasilisk.String(): 0,
config.HFCockatrice.String(): 0,
}
c.ReservedAttributes = false
})
eBad := neotest.NewExecutor(t, bcBad, validatorBad, committeeBad)
Expand All @@ -2069,7 +2074,7 @@ func TestBlockchain_VerifyTx(t *testing.T) {
eBad.SignTx(t, tx, 1_0000_0000, eBad.Committee)
err := bcBad.VerifyTx(tx)
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "invalid attribute: NotaryAssisted attribute was found, but P2PSigExtensions are disabled"))
require.True(t, strings.Contains(err.Error(), "invalid attribute: NotaryAssisted attribute was found, but Echidna is not active yet"))
})
t.Run("Enabled, insufficient network fee", func(t *testing.T) {
tx := getNotaryAssistedTx(e, 1, 0)
Expand Down Expand Up @@ -2572,7 +2577,7 @@ func TestBlockchain_GenesisTransactionExtension(t *testing.T) {
// in the right order.
func TestNativenames(t *testing.T) {
bc, _ := chain.NewSingleWithCustomConfig(t, func(cfg *config.Blockchain) {
cfg.Hardforks = map[string]uint32{}
cfg.Hardforks = nil // default (all hardforks enabled) behaviour.
cfg.P2PSigExtensions = true
})
natives := bc.GetNatives()
Expand Down
2 changes: 1 addition & 1 deletion pkg/core/interop/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
}
md, ok := c.mdCache[key]
if !ok {
panic(fmt.Errorf("native contract descriptor cache is not initialized: contract %s, hardfork %s", c.Hash.StringLE(), key))
panic(fmt.Errorf("native contract descriptor cache is not initialized: contract %s (%s), hardfork %s", c.Hash.StringLE(), c.Name, key))

Check warning on line 252 in pkg/core/interop/context.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/interop/context.go#L252

Added line #L252 was not covered by tests
}
if md == nil {
panic(fmt.Errorf("native contract descriptor cache is nil: contract %s, hardfork %s", c.Hash.StringLE(), key))
Expand Down
20 changes: 9 additions & 11 deletions pkg/core/native/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ func NewContracts(cfg config.ProtocolConfiguration) *Contracts {
cs.Ledger = ledger
cs.Contracts = append(cs.Contracts, ledger)

gas := newGAS(int64(cfg.InitialGASSupply), cfg.P2PSigExtensions)
gas := newGAS(int64(cfg.InitialGASSupply))
neo := newNEO(cfg)
policy := newPolicy(cfg.P2PSigExtensions)
policy := newPolicy()
neo.GAS = gas
neo.Policy = policy
gas.NEO = neo
Expand All @@ -100,15 +100,13 @@ func NewContracts(cfg config.ProtocolConfiguration) *Contracts {
cs.Oracle = oracle
cs.Contracts = append(cs.Contracts, oracle)

if cfg.P2PSigExtensions {
notary := newNotary()
notary.GAS = gas
notary.NEO = neo
notary.Desig = desig
notary.Policy = policy
cs.Notary = notary
cs.Contracts = append(cs.Contracts, notary)
}
notary := newNotary()
notary.GAS = gas
notary.NEO = neo
notary.Desig = desig
notary.Policy = policy
cs.Notary = notary
cs.Contracts = append(cs.Contracts, notary)

return cs
}
Expand Down
8 changes: 5 additions & 3 deletions pkg/core/native/management.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,6 @@
for _, hf := range config.Hardforks {
if _, ok := activeHFs[hf]; ok && ic.IsHardforkActivation(hf) {
isUpdate = true
activation := hf // avoid loop variable pointer exporting.
activeIn = &activation // reuse ActiveIn variable for the initialization hardfork.
// Break immediately since native Initialize should be called starting from the first hardfork in a raw
// (if there are multiple hardforks with the same enabling height).
break
Expand All @@ -626,6 +624,10 @@
currentActiveHFs = append(currentActiveHFs, hf)
}
}
// activeIn is not included into the activeHFs list.
if activeIn != nil && activeIn.Cmp(latestHF) > 0 {
latestHF = *activeIn
}
if !(isDeploy || isUpdate) {
continue
}
Expand Down Expand Up @@ -668,7 +670,7 @@
// The rest of activating hardforks also require initialization.
for _, hf := range currentActiveHFs {
if err := native.Initialize(ic, &hf, hfSpecificMD); err != nil {
return fmt.Errorf("initializing %s native contract at HF %d: %w", md.Name, activeIn, err)
return fmt.Errorf("initializing %s native contract at HF %d: %w", md.Name, hf, err)

Check warning on line 673 in pkg/core/native/management.go

View check run for this annotation

Codecov / codecov/patch

pkg/core/native/management.go#L673

Added line #L673 was not covered by tests
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/core/native/management_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

func TestDeployGetUpdateDestroyContract(t *testing.T) {
mgmt := newManagement()
mgmt.Policy = newPolicy(false)
mgmt.Policy = newPolicy()
d := dao.NewSimple(storage.NewMemoryStore(), false)
ic := &interop.Context{DAO: d}
err := mgmt.Initialize(ic, nil, nil)
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestManagement_Initialize(t *testing.T) {

func TestManagement_GetNEP17Contracts(t *testing.T) {
mgmt := newManagement()
mgmt.Policy = newPolicy(false)
mgmt.Policy = newPolicy()
d := dao.NewSimple(storage.NewMemoryStore(), false)
err := mgmt.Initialize(&interop.Context{DAO: d}, nil, nil)
require.NoError(t, err)
Expand Down
Loading
Loading