diff --git a/docs/notary.md b/docs/notary.md index b75d40a45..bd68bef1c 100644 --- a/docs/notary.md +++ b/docs/notary.md @@ -81,7 +81,7 @@ It exposes several methods to the outside world: | Method | Parameters | Return value | Description | | --- | --- | --- | --- | | `onNEP17Payment` | `from` (uint160) - GAS sender account.
`amount` (int) - amount of GAS to deposit.
`data` represents array of two parameters:
1. `to` (uint160) - account of the deposit owner.
2. `till` (int) - deposit lock height. | `bool` | Automatically called after GAS transfer to Notary native contract address and records deposited amount as belonging to `to` address with a lock till `till` chain's height. Can only be invoked from native GAS contract. Must be witnessed by `from`. `to` can be left unspecified (null), with a meaning that `to` is the same address as `from`. `amount` can't be less than 2×`FEE` for the first deposit call for the `to` address. Each successive deposit call must have `till` value equal to or more than the previous successful call (allowing for renewal), if it has additional amount of GAS it adds up to the already deposited value.| -| `lockDepositUntil` | `address` (uint160) - account of the deposit owner.
`till` (int) - new height deposit is valid until (can't be less than previous value). | `void` | Updates deposit expiration value. Must be witnessed by `address`. | +| `lockDepositUntil` | `address` (uint160) - account of the deposit owner.
`till` (int) - new height deposit is valid until (can't be less than previous value and can't be less than the height when transaction is accepted to the chain plus one). | `void` | Updates deposit expiration value. Must be witnessed by `address`. | | `withdraw` | `from` (uint160) - account of the deposit owner.
`to` (uint160) - account to transfer GAS to. | `bool` | Sends all deposited GAS for `from` address to `to` address. Must be witnessed by `from`. `to` can be left unspecified (null), with a meaning that `to` is the same address as `from`. It can only be successful if the lock has already expired, attempting to withdraw the deposit before that height fails. Partial withdrawal is not supported. Returns boolean result, `true` for successful calls and `false` for failed ones. | | `balanceOf` | `addr` (uint160) - account of the deposit owner. | `int` | Returns deposited GAS amount for specified address (integer). | | `expirationOf` | `addr` (uint160) - account of the deposit owner. | `int` | Returns deposit lock height for specified address (integer). | diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index 778a1cade..0aa71c29a 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -371,7 +371,9 @@ func (ic *Context) Exec() error { return ic.VM.Run() } -// BlockHeight returns current block height got from Context's block if it's set. +// BlockHeight returns the latest persisted and stored block height/index. +// Persisting block index is not taken into account. If Context's block is set, +// then BlockHeight calculations relies on persisting block index. func (ic *Context) BlockHeight() uint32 { if ic.Block != nil { return ic.Block.Index - 1 // Persisting block is not yet stored. @@ -393,7 +395,7 @@ func (ic *Context) GetBlock(hash util.Uint256) (*block.Block, error) { if err != nil { return nil, err } - if block.Index > ic.BlockHeight() { + if block.Index > ic.BlockHeight() { // persisting block is not reachable. return nil, storage.ErrKeyNotFound } return block, nil @@ -403,7 +405,7 @@ func (ic *Context) GetBlock(hash util.Uint256) (*block.Block, error) { func (ic *Context) IsHardforkEnabled(hf config.Hardfork) bool { height, ok := ic.Hardforks[hf.String()] if ok { - return ic.BlockHeight() >= height + return (ic.BlockHeight() + 1) >= height // persisting block should be taken into account. } // Completely rely on proper hardforks initialisation made by core.NewBlockchain. return false diff --git a/pkg/core/interop/contract/account_test.go b/pkg/core/interop/contract/account_test.go index bedb97730..e03a4ea1e 100644 --- a/pkg/core/interop/contract/account_test.go +++ b/pkg/core/interop/contract/account_test.go @@ -117,11 +117,12 @@ func TestCreateMultisigAccount(t *testing.T) { }) } -func TestCreateAccount_Hardfork(t *testing.T) { +func TestCreateAccount_HFAspidochelone(t *testing.T) { + const enabledHeight = 3 bc, acc := chain.NewSingleWithCustomConfig(t, func(c *config.Blockchain) { c.P2PSigExtensions = true // `basicchain.Init` requires Notary enabled c.Hardforks = map[string]uint32{ - config.HFAspidochelone.String(): 2, + config.HFAspidochelone.String(): enabledHeight, } }) e := neotest.NewExecutor(t, bc, acc, acc) @@ -161,6 +162,7 @@ func TestCreateAccount_Hardfork(t *testing.T) { e.CheckHalt(t, tx2Multisig.Hash()) // block #3: updated prices (larger than the previous ones) + require.Equal(t, uint32(enabledHeight-1), e.Chain.BlockHeight()) tx3Standard := createAccTx(t, standardScript) tx3Multisig := createAccTx(t, multisigScript) e.AddNewBlock(t, tx3Standard, tx3Multisig) diff --git a/pkg/core/interop/runtime/ext_test.go b/pkg/core/interop/runtime/ext_test.go index c0764fd9f..d33c1043f 100644 --- a/pkg/core/interop/runtime/ext_test.go +++ b/pkg/core/interop/runtime/ext_test.go @@ -691,11 +691,14 @@ func TestNotify(t *testing.T) { } func TestSystemRuntimeNotify_HFBasilisk(t *testing.T) { - const ntfName = "Hello, world!" + const ( + ntfName = "Hello, world!" + enabledHeight = 3 + ) bc, acc := chain.NewSingleWithCustomConfig(t, func(c *config.Blockchain) { c.Hardforks = map[string]uint32{ - config.HFBasilisk.String(): 2, + config.HFBasilisk.String(): enabledHeight, } }) e := neotest.NewExecutor(t, bc, acc, acc) @@ -738,6 +741,8 @@ func TestSystemRuntimeNotify_HFBasilisk(t *testing.T) { } ctrInv := e.NewInvoker(ctr.Hash, e.Validator) + // Block 0 is genesis. + // Block 1: deploy contract. e.DeployContract(t, ctr, nil) @@ -745,6 +750,7 @@ func TestSystemRuntimeNotify_HFBasilisk(t *testing.T) { ctrInv.Invoke(t, nil, "main") // Block 3: bad event should fault the execution. + require.Equal(t, uint32(enabledHeight-1), e.Chain.BlockHeight()) ctrInv.InvokeFail(t, "System.Runtime.Notify failed: notification Hello, world! is invalid: parameter 0 type mismatch: Integer (manifest) vs Boolean (notification)", "main") diff --git a/pkg/core/native/designate.go b/pkg/core/native/designate.go index 9cac6c7c1..0e295d593 100644 --- a/pkg/core/native/designate.go +++ b/pkg/core/native/designate.go @@ -204,7 +204,7 @@ func (s *Designate) getDesignatedByRole(ic *interop.Context, args []stackitem.It panic(ErrInvalidIndex) } index := ind.Uint64() - if index > uint64(ic.BlockHeight()+1) { + if index > uint64(ic.BlockHeight()+1) { // persisting block should be taken into account. panic(ErrInvalidIndex) } pubs, _, err := s.GetDesignatedByRole(ic.DAO, r, uint32(index)) diff --git a/pkg/core/native/interop.go b/pkg/core/native/interop.go index 00f0f3c73..7f250b4a7 100644 --- a/pkg/core/native/interop.go +++ b/pkg/core/native/interop.go @@ -34,7 +34,7 @@ func Call(ic *interop.Context) error { if len(history) == 0 { return fmt.Errorf("native contract %s is disabled", meta.Name) } - if history[0] > ic.BlockHeight() { + if history[0] > ic.BlockHeight() { // persisting block must not be taken into account. return fmt.Errorf("native contract %s is active after height = %d", meta.Name, history[0]) } m, ok := meta.GetMethodByOffset(ic.VM.Context().IP()) diff --git a/pkg/core/native/native_test/gas_test.go b/pkg/core/native/native_test/gas_test.go index e92f9097b..05a2f83dd 100644 --- a/pkg/core/native/native_test/gas_test.go +++ b/pkg/core/native/native_test/gas_test.go @@ -90,9 +90,9 @@ func TestGAS_RewardWithP2PSigExtensionsEnabled(t *testing.T) { e.CheckGASBalance(t, notaryNode.GetScriptHash(), big.NewInt(0)) } - // deposit GAS for `signer` with lock until the next block + // deposit GAS for `signer` with lock until the next block (inclusively) depositAmount := 100_0000 + (2+int64(nKeys))*notaryServiceFeePerKey // sysfee + netfee of the next transaction - gasCommitteeInvoker.Invoke(t, true, "transfer", e.CommitteeHash, notaryHash, depositAmount, []any{e.CommitteeHash, e.Chain.BlockHeight() + 1}) + gasCommitteeInvoker.Invoke(t, true, "transfer", e.CommitteeHash, notaryHash, depositAmount, []any{e.CommitteeHash, e.Chain.BlockHeight() + 2}) // save initial GAS total supply getGASTS := func(t *testing.T) int64 { diff --git a/pkg/core/native/native_test/notary_test.go b/pkg/core/native/native_test/notary_test.go index 1f846cd3e..9f08f6333 100644 --- a/pkg/core/native/native_test/notary_test.go +++ b/pkg/core/native/native_test/notary_test.go @@ -197,12 +197,12 @@ func TestNotary_NotaryNodesReward(t *testing.T) { e.CheckGASBalance(t, notaryNode.GetScriptHash(), big.NewInt(0)) } - // deposit GAS for `signer` with lock until the next block + // deposit GAS for `signer` with lock until the next block inclusively depositAmount := 100_0000 + (2+int64(nKeys))*feePerKey // sysfee + netfee of the next transaction if !spendFullDeposit { depositAmount += 1_0000 } - gasCommitteeInvoker.Invoke(t, true, "transfer", multisigHash, notaryHash, depositAmount, ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: e.Chain.BlockHeight() + 1}) + gasCommitteeInvoker.Invoke(t, true, "transfer", multisigHash, notaryHash, depositAmount, ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: e.Chain.BlockHeight() + 2}) // send transaction with Notary contract as a sender tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1_000_000) diff --git a/pkg/core/native/notary.go b/pkg/core/native/notary.go index f58e628a7..75224f2d0 100644 --- a/pkg/core/native/notary.go +++ b/pkg/core/native/notary.go @@ -232,8 +232,8 @@ func (n *Notary) onPayment(ic *interop.Context, args []stackitem.Item) stackitem currentHeight := ic.BlockHeight() deposit := n.GetDepositFor(ic.DAO, to) till := toUint32(additionalParams[1]) - if till < currentHeight { - panic(fmt.Errorf("`till` shouldn't be less then the chain's height %d", currentHeight)) + if till < currentHeight+2 { + panic(fmt.Errorf("`till` shouldn't be less then the chain's height + 1 (%d at min)", currentHeight+2)) } if deposit != nil && till < deposit.Till { panic(fmt.Errorf("`till` shouldn't be less then the previous value %d", deposit.Till)) @@ -272,7 +272,7 @@ func (n *Notary) lockDepositUntil(ic *interop.Context, args []stackitem.Item) st return stackitem.NewBool(false) } till := toUint32(args[1]) - if till < ic.BlockHeight() { + if till < (ic.BlockHeight() + 1 + 1) { // deposit can't expire at the current persisting block. return stackitem.NewBool(false) } deposit := n.GetDepositFor(ic.DAO, addr) @@ -308,6 +308,7 @@ func (n *Notary) withdraw(ic *interop.Context, args []stackitem.Item) stackitem. if deposit == nil { return stackitem.NewBool(false) } + // Allow withdrawal only after `till` block was persisted, thus, use ic.BlockHeight(). if ic.BlockHeight() < deposit.Till { return stackitem.NewBool(false) }