Merge pull request #1593 from nspcc-dev/notary_contract_fix

core: allow to change deposit's `till` for owner only
nspcc-dev · Dec 8, 2020 · 8ed1d4d · 8ed1d4d
2 parents fb13aca + 74a143c
commit 8ed1d4d
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 8 deletions.
diff --git a/pkg/core/native/notary.go b/pkg/core/native/notary.go
@@ -33,7 +33,8 @@ const (
 	notaryContractID = reservedContractID - 1
 
 	// prefixDeposit is a prefix for storing Notary deposits.
-	prefixDeposit = 1
+	prefixDeposit           = 1
+	defaultDepositDeltaTill = 5760
 )
 
 // newNotary returns Notary native contract.
@@ -158,24 +159,29 @@ func (n *Notary) onPayment(ic *interop.Context, args []stackitem.Item) stackitem
 	if !additionalParams[0].Equals(stackitem.Null{}) {
 		to = toUint160(additionalParams[0])
 	}
-	till := toUint32(additionalParams[1])
+
+	allowedChangeTill := ic.Tx.Sender() == to
 	currentHeight := ic.Chain.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))
 	}
-
-	deposit := n.getDepositFor(ic.DAO, to)
+	if deposit != nil && till < deposit.Till {
+		panic(fmt.Errorf("`till` shouldn't be less then the previous value %d", deposit.Till))
+	}
 	if deposit == nil {
 		if amount.Cmp(big.NewInt(2*transaction.NotaryServiceFeePerKey)) < 0 {
 			panic(fmt.Errorf("first deposit can not be less then %d, got %d", 2*transaction.NotaryServiceFeePerKey, amount.Int64()))
 		}
 		deposit = &state.Deposit{
 			Amount: new(big.Int),
 		}
-	} else {
-		if till < deposit.Till {
-			panic(fmt.Errorf("`till` shouldn't be less then the previous value %d", deposit.Till))
+		if !allowedChangeTill {
+			till = currentHeight + defaultDepositDeltaTill
 		}
+	} else if !allowedChangeTill { // only deposit's owner is allowed to set or update `till`
+		till = deposit.Till
 	}
 	deposit.Amount.Add(deposit.Amount, amount)
 	deposit.Till = till

diff --git a/pkg/core/native_notary_test.go b/pkg/core/native_notary_test.go
@@ -1,6 +1,7 @@
 package core
 
 import (
+	"math"
 	"testing"
 
 	"github.com/nspcc-dev/neo-go/internal/testchain"
@@ -24,7 +25,7 @@ func TestNotaryContractPipeline(t *testing.T) {
 
 	notaryHash := chain.contracts.Notary.Hash
 	gasHash := chain.contracts.GAS.Hash
-	depositLock := 30
+	depositLock := 100
 
 	// check Notary contract has no GAS on the account
 	checkBalanceOf(t, chain, notaryHash, 0)
@@ -217,6 +218,28 @@ func TestNotaryContractPipeline(t *testing.T) {
 	withdrawRes, err = invokeContractMethod(chain, 100000000, notaryHash, "withdraw", testchain.MultisigScriptHash(), testchain.MultisigScriptHash())
 	require.NoError(t, err)
 	checkResult(t, withdrawRes, stackitem.NewBool(false))
+
+	// `onPayment`: good first deposit to other account, should set default `till` even if other `till` value is provided
+	transferTx = transferTokenFromMultisigAccount(t, chain, notaryHash, gasHash, 2*transaction.NotaryServiceFeePerKey, acc.PrivateKey().PublicKey().GetScriptHash(), int64(math.MaxUint32-1))
+	res, err = chain.GetAppExecResults(transferTx.Hash(), trigger.Application)
+	require.NoError(t, err)
+	require.Equal(t, vm.HaltState, res[0].VMState)
+	require.Equal(t, 0, len(res[0].Stack))
+	checkBalanceOf(t, chain, notaryHash, 2*transaction.NotaryServiceFeePerKey)
+	till, err = invokeContractMethod(chain, 100000000, notaryHash, "expirationOf", acc.PrivateKey().PublicKey().GetScriptHash())
+	require.NoError(t, err)
+	checkResult(t, till, stackitem.Make(5760+chain.BlockHeight()-2))
+
+	// `onPayment`: good second deposit to other account, shouldn't update `till` even if other `till` value is provided
+	transferTx = transferTokenFromMultisigAccount(t, chain, notaryHash, gasHash, 2*transaction.NotaryServiceFeePerKey, acc.PrivateKey().PublicKey().GetScriptHash(), int64(math.MaxUint32-1))
+	res, err = chain.GetAppExecResults(transferTx.Hash(), trigger.Application)
+	require.NoError(t, err)
+	require.Equal(t, vm.HaltState, res[0].VMState)
+	require.Equal(t, 0, len(res[0].Stack))
+	checkBalanceOf(t, chain, notaryHash, 4*transaction.NotaryServiceFeePerKey)
+	till, err = invokeContractMethod(chain, 100000000, notaryHash, "expirationOf", acc.PrivateKey().PublicKey().GetScriptHash())
+	require.NoError(t, err)
+	checkResult(t, till, stackitem.Make(5760+chain.BlockHeight()-4))
 }
 
 func TestNotaryNodesReward(t *testing.T) {