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..25cae19d8 100644 --- a/pkg/core/native/native_test/notary_test.go +++ b/pkg/core/native/native_test/notary_test.go @@ -79,13 +79,13 @@ func TestNotary_Pipeline(t *testing.T) { neoCommitteeInvoker.InvokeFail(t, "only GAS can be accepted for deposit", "transfer", multisigHash, notaryHash, int64(1), ¬ary.OnNEP17PaymentData{Till: uint32(depositLock)}) // `onPayment`: insufficient first deposit - gasCommitteeInvoker.InvokeFail(t, "first deposit can not be less then", "transfer", multisigHash, notaryHash, int64(2*feePerKey-1), ¬ary.OnNEP17PaymentData{Till: uint32(depositLock)}) + gasCommitteeInvoker.InvokeFail(t, "first deposit can not be less than", "transfer", multisigHash, notaryHash, int64(2*feePerKey-1), ¬ary.OnNEP17PaymentData{Till: uint32(depositLock)}) // `onPayment`: invalid `data` (missing `till` parameter) gasCommitteeInvoker.InvokeFail(t, "`data` parameter should be an array of 2 elements", "transfer", multisigHash, notaryHash, 2*feePerKey, []any{nil}) // `onPayment`: invalid `data` (outdated `till` parameter) - gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less then the chain's height", "transfer", multisigHash, notaryHash, 2*feePerKey, ¬ary.OnNEP17PaymentData{}) + gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less than the chain's height", "transfer", multisigHash, notaryHash, 2*feePerKey, ¬ary.OnNEP17PaymentData{}) // `onPayment`: good gasCommitteeInvoker.Invoke(t, true, "transfer", multisigHash, notaryHash, 2*feePerKey, ¬ary.OnNEP17PaymentData{Till: uint32(depositLock)}) @@ -107,13 +107,13 @@ func TestNotary_Pipeline(t *testing.T) { // `expirationOf`: check `till` is updated. notaryCommitteeInvoker.Invoke(t, depositLock+1, "expirationOf", multisigHash) - // `onPayment`: empty payment, should fail because `till` less then the previous one - gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less then the previous value", "transfer", multisigHash, notaryHash, int64(0), ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: uint32(depositLock)}) + // `onPayment`: empty payment, should fail because `till` less than the previous one + gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less than the previous value", "transfer", multisigHash, notaryHash, int64(0), ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: uint32(depositLock)}) checkBalanceOf(t, notaryHash, 3*feePerKey) notaryCommitteeInvoker.Invoke(t, depositLock+1, "expirationOf", multisigHash) - // `onPayment`: empty payment, should fail because `till` less then the chain height - gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less then the chain's height", "transfer", multisigHash, notaryHash, int64(0), ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: uint32(1)}) + // `onPayment`: empty payment, should fail because `till` less than the chain height + gasCommitteeInvoker.InvokeFail(t, "`till` shouldn't be less than the chain's height", "transfer", multisigHash, notaryHash, int64(0), ¬ary.OnNEP17PaymentData{Account: &multisigHash, Till: uint32(1)}) checkBalanceOf(t, notaryHash, 3*feePerKey) notaryCommitteeInvoker.Invoke(t, depositLock+1, "expirationOf", multisigHash) @@ -126,11 +126,11 @@ func TestNotary_Pipeline(t *testing.T) { notaryCommitteeInvoker.Invoke(t, false, "lockDepositUntil", util.Uint160{1, 2, 3}, int64(depositLock+3)) notaryCommitteeInvoker.Invoke(t, depositLock+2, "expirationOf", multisigHash) - // `lockDepositUntil`: bad `till` (less then the previous one) + // `lockDepositUntil`: bad `till` (less than the previous one) notaryCommitteeInvoker.Invoke(t, false, "lockDepositUntil", multisigHash, int64(depositLock+1)) notaryCommitteeInvoker.Invoke(t, depositLock+2, "expirationOf", multisigHash) - // `lockDepositUntil`: bad `till` (less then the chain's height) + // `lockDepositUntil`: bad `till` (less than the chain's height) notaryCommitteeInvoker.Invoke(t, false, "lockDepositUntil", multisigHash, int64(1)) notaryCommitteeInvoker.Invoke(t, depositLock+2, "expirationOf", multisigHash) @@ -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..c0b2da285 100644 --- a/pkg/core/native/notary.go +++ b/pkg/core/native/notary.go @@ -232,16 +232,16 @@ 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 than 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)) + panic(fmt.Errorf("`till` shouldn't be less than the previous value %d", deposit.Till)) } feePerKey := n.GetNotaryServiceFeePerKey(ic.DAO) if deposit == nil { if amount.Cmp(big.NewInt(2*feePerKey)) < 0 { - panic(fmt.Errorf("first deposit can not be less then %d, got %d", 2*feePerKey, amount.Int64())) + panic(fmt.Errorf("first deposit can not be less than %d, got %d", 2*feePerKey, amount.Int64())) } deposit = &state.Deposit{ Amount: new(big.Int), @@ -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) } diff --git a/pkg/services/rpcsrv/server.go b/pkg/services/rpcsrv/server.go index d02588b7b..85e32069c 100644 --- a/pkg/services/rpcsrv/server.go +++ b/pkg/services/rpcsrv/server.go @@ -2998,7 +2998,7 @@ func (s *Server) blockHeightFromParam(param *params.Param) (uint32, *neorpc.Erro } if num < 0 || int64(num) > int64(s.chain.BlockHeight()) { - return 0, neorpc.WrapErrorWithData(neorpc.ErrUnknownHeight, fmt.Sprintf("param at index %d should be greater than or equal to 0 and less then or equal to current block height, got: %d", 0, num)) + return 0, neorpc.WrapErrorWithData(neorpc.ErrUnknownHeight, fmt.Sprintf("param at index %d should be greater than or equal to 0 and less than or equal to current block height, got: %d", 0, num)) } return uint32(num), nil }