From cb5ecaefe7c220409c93c96bee2ffc780524e0b7 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 13 Dec 2020 23:30:21 +0300 Subject: [PATCH] native: drop OnPersistEnd Now that PostPersist is being run for every native contract we can do our dirty caching tricks right there instead of OnPersistEnd. --- pkg/core/blockchain.go | 15 --------------- pkg/core/blockchain_test.go | 2 -- pkg/core/native/designate.go | 7 +------ pkg/core/native/native_neo.go | 21 ++++++++------------- pkg/core/native/notary.go | 15 +++++---------- pkg/core/native/policy.go | 19 +++++++------------ pkg/core/native_designate_test.go | 3 --- pkg/core/native_neo_test.go | 2 -- pkg/core/native_oracle_test.go | 1 - 9 files changed, 21 insertions(+), 64 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 99cd714dd..443f8609c 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -724,21 +724,6 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error bc.lock.Unlock() return err } - if err := bc.contracts.Policy.OnPersistEnd(bc.dao); err != nil { - bc.lock.Unlock() - return fmt.Errorf("failed to call OnPersistEnd for Policy native contract: %w", err) - } - if bc.P2PSigExtensionsEnabled() { - err := bc.contracts.Notary.OnPersistEnd(bc.dao) - if err != nil { - bc.lock.Unlock() - return fmt.Errorf("failed to call OnPersistEnd for Notary native contract: %w", err) - } - } - if err := bc.contracts.Designate.OnPersistEnd(bc.dao); err != nil { - bc.lock.Unlock() - return err - } bc.dao.MPT.Flush() // Every persist cycle we also compact our in-memory MPT. persistedHeight := atomic.LoadUint32(&bc.persistedHeight) diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go index ea3fbb437..d8b892ffb 100644 --- a/pkg/core/blockchain_test.go +++ b/pkg/core/blockchain_test.go @@ -540,7 +540,6 @@ func TestVerifyTx(t *testing.T) { ic.SpawnVM() ic.VM.LoadScript([]byte{byte(opcode.RET)}) require.NoError(t, bc.contracts.Designate.DesignateAsRole(ic, native.RoleOracle, oraclePubs)) - require.NoError(t, bc.contracts.Designate.OnPersistEnd(ic.DAO)) _, err = ic.DAO.Persist() require.NoError(t, err) @@ -747,7 +746,6 @@ func TestVerifyTx(t *testing.T) { ic.SpawnVM() ic.VM.LoadScript([]byte{byte(opcode.RET)}) require.NoError(t, bc.contracts.Designate.DesignateAsRole(ic, native.RoleP2PNotary, keys.PublicKeys{notary.PrivateKey().PublicKey()})) - require.NoError(t, bc.contracts.Designate.OnPersistEnd(ic.DAO)) _, err = ic.DAO.Persist() require.NoError(t, err) getNotaryAssistedTx := func(signaturesCount uint8, serviceFee int64) *transaction.Transaction { diff --git a/pkg/core/native/designate.go b/pkg/core/native/designate.go index 57c726cfa..ecfbdbd77 100644 --- a/pkg/core/native/designate.go +++ b/pkg/core/native/designate.go @@ -102,16 +102,11 @@ func (s *Designate) OnPersist(ic *interop.Context) error { // PostPersist implements Contract interface. func (s *Designate) PostPersist(ic *interop.Context) error { - return nil -} - -// OnPersistEnd updates cached values if they've been changed. -func (s *Designate) OnPersistEnd(d dao.DAO) error { if !s.rolesChanged() { return nil } - nodeKeys, height, err := s.GetDesignatedByRole(d, RoleOracle, math.MaxUint32) + nodeKeys, height, err := s.GetDesignatedByRole(ic.DAO, RoleOracle, math.MaxUint32) if err != nil { return err } diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index ed5ce11da..739264680 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -320,7 +320,14 @@ func (n *NEO) PostPersist(ic *interop.Context) error { } } - n.OnPersistEnd(ic.DAO) + if n.gasPerBlockChanged.Load().(bool) { + gr, err := n.getSortedGASRecordFromDAO(ic.DAO) + if err != nil { + panic(err) + } + n.gasPerBlock.Store(gr) + n.gasPerBlockChanged.Store(false) + } return nil } @@ -347,18 +354,6 @@ func (n *NEO) getGASPerVote(d dao.DAO, key []byte, index ...uint32) []big.Int { return reward } -// OnPersistEnd updates cached values if they've been changed. -func (n *NEO) OnPersistEnd(d dao.DAO) { - if n.gasPerBlockChanged.Load().(bool) { - gr, err := n.getSortedGASRecordFromDAO(d) - if err != nil { - panic(err) - } - n.gasPerBlock.Store(gr) - n.gasPerBlockChanged.Store(false) - } -} - func (n *NEO) increaseBalance(ic *interop.Context, h util.Uint160, si *state.StorageItem, amount *big.Int) error { acc, err := state.NEOBalanceStateFromBytes(si.Value) if err != nil { diff --git a/pkg/core/native/notary.go b/pkg/core/native/notary.go index 5ac5a393f..110355688 100644 --- a/pkg/core/native/notary.go +++ b/pkg/core/native/notary.go @@ -158,24 +158,19 @@ func (n *Notary) OnPersist(ic *interop.Context) error { return nil } -// OnPersistEnd updates cached Policy values if they've been changed -func (n *Notary) OnPersistEnd(dao dao.DAO) error { +// PostPersist implements Contract interface. +func (n *Notary) PostPersist(ic *interop.Context) error { + n.lock.Lock() + defer n.lock.Unlock() if n.isValid { return nil } - n.lock.Lock() - defer n.lock.Unlock() - n.maxNotValidBeforeDelta = getUint32WithKey(n.ContractID, dao, maxNotValidBeforeDeltaKey, defaultMaxNotValidBeforeDelta) + n.maxNotValidBeforeDelta = getUint32WithKey(n.ContractID, ic.DAO, maxNotValidBeforeDeltaKey, defaultMaxNotValidBeforeDelta) n.isValid = true return nil } -// PostPersist implements Contract interface. -func (n *Notary) PostPersist(ic *interop.Context) error { - return nil -} - // onPayment records deposited amount as belonging to "from" address with a lock // till the specified chain's height. func (n *Notary) onPayment(ic *interop.Context, args []stackitem.Item) stackitem.Item { diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 33d5f14a9..192aa7728 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -152,25 +152,20 @@ func (p *Policy) OnPersist(ic *interop.Context) error { // PostPersist implements Contract interface. func (p *Policy) PostPersist(ic *interop.Context) error { - return nil -} - -// OnPersistEnd updates cached Policy values if they've been changed -func (p *Policy) OnPersistEnd(dao dao.DAO) error { + p.lock.Lock() + defer p.lock.Unlock() if p.isValid { return nil } - p.lock.Lock() - defer p.lock.Unlock() - p.maxTransactionsPerBlock = getUint32WithKey(p.ContractID, dao, maxTransactionsPerBlockKey, defaultMaxTransactionsPerBlock) - p.maxBlockSize = getUint32WithKey(p.ContractID, dao, maxBlockSizeKey, defaultMaxBlockSize) - p.feePerByte = getInt64WithKey(p.ContractID, dao, feePerByteKey, defaultFeePerByte) - p.maxBlockSystemFee = getInt64WithKey(p.ContractID, dao, maxBlockSystemFeeKey, defaultMaxBlockSystemFee) + p.maxTransactionsPerBlock = getUint32WithKey(p.ContractID, ic.DAO, maxTransactionsPerBlockKey, defaultMaxTransactionsPerBlock) + p.maxBlockSize = getUint32WithKey(p.ContractID, ic.DAO, maxBlockSizeKey, defaultMaxBlockSize) + p.feePerByte = getInt64WithKey(p.ContractID, ic.DAO, feePerByteKey, defaultFeePerByte) + p.maxBlockSystemFee = getInt64WithKey(p.ContractID, ic.DAO, maxBlockSystemFeeKey, defaultMaxBlockSystemFee) p.maxVerificationGas = defaultMaxVerificationGas p.blockedAccounts = make([]util.Uint160, 0) - siMap, err := dao.GetStorageItemsWithPrefix(p.ContractID, []byte{blockedAccountPrefix}) + siMap, err := ic.DAO.GetStorageItemsWithPrefix(p.ContractID, []byte{blockedAccountPrefix}) if err != nil { return fmt.Errorf("failed to get blocked accounts from storage: %w", err) } diff --git a/pkg/core/native_designate_test.go b/pkg/core/native_designate_test.go index d318c8772..b381a923f 100644 --- a/pkg/core/native_designate_test.go +++ b/pkg/core/native_designate_test.go @@ -134,7 +134,6 @@ func TestDesignate_DesignateAsRole(t *testing.T) { setSigner(tx, testchain.CommitteeScriptHash()) err = des.DesignateAsRole(ic, native.RoleOracle, keys.PublicKeys{pub}) require.NoError(t, err) - require.NoError(t, des.OnPersistEnd(ic.DAO)) pubs, index, err = des.GetDesignatedByRole(ic.DAO, native.RoleOracle, bl.Index+1) require.NoError(t, err) @@ -152,7 +151,6 @@ func TestDesignate_DesignateAsRole(t *testing.T) { pub1 := priv.PublicKey() err = des.DesignateAsRole(ic, native.RoleStateValidator, keys.PublicKeys{pub1}) require.NoError(t, err) - require.NoError(t, des.OnPersistEnd(ic.DAO)) pubs, index, err = des.GetDesignatedByRole(ic.DAO, native.RoleOracle, 255) require.NoError(t, err) @@ -172,7 +170,6 @@ func TestDesignate_DesignateAsRole(t *testing.T) { err = des.DesignateAsRole(ic, native.RoleP2PNotary, keys.PublicKeys{pub1}) require.NoError(t, err) - require.NoError(t, des.OnPersistEnd(ic.DAO)) pubs, index, err = des.GetDesignatedByRole(ic.DAO, native.RoleP2PNotary, 255) require.NoError(t, err) diff --git a/pkg/core/native_neo_test.go b/pkg/core/native_neo_test.go index 4ef8759d4..ee8be24e7 100644 --- a/pkg/core/native_neo_test.go +++ b/pkg/core/native_neo_test.go @@ -225,7 +225,6 @@ func TestNEO_SetGasPerBlock(t *testing.T) { ok, err := neo.SetGASPerBlock(ic, 10, big.NewInt(native.GASFactor*2)) require.NoError(t, err) require.True(t, ok) - neo.OnPersistEnd(ic.DAO) _, err = ic.DAO.Persist() require.NoError(t, err) @@ -242,7 +241,6 @@ func TestNEO_SetGasPerBlock(t *testing.T) { }) }) - neo.OnPersistEnd(ic.DAO) g := neo.GetGASPerBlock(ic.DAO, 9) require.EqualValues(t, 5*native.GASFactor, g.Int64()) diff --git a/pkg/core/native_oracle_test.go b/pkg/core/native_oracle_test.go index d914d579b..80555bb15 100644 --- a/pkg/core/native_oracle_test.go +++ b/pkg/core/native_oracle_test.go @@ -144,7 +144,6 @@ func TestOracle_Request(t *testing.T) { ic.VM.LoadScript([]byte{byte(opcode.RET)}) err = bc.contracts.Designate.DesignateAsRole(ic, native.RoleOracle, keys.PublicKeys{pub}) require.NoError(t, err) - require.NoError(t, bc.contracts.Designate.OnPersistEnd(ic.DAO)) tx = transaction.New(netmode.UnitTestNet, native.GetOracleResponseScript(), 0) ic.Tx = tx