From d7c0785148dd4da245a8653bc8f8c328247318f3 Mon Sep 17 00:00:00 2001 From: Artyom Sayadyan Date: Wed, 13 Dec 2023 14:47:36 +0300 Subject: [PATCH] NODE-2640 Fixed estimation with lazy refsCosts in EstimatorContext (#3929) --- .../v1/estimator/v3/EstimatorContext.scala | 2 +- .../v1/estimator/v3/ScriptEstimatorV3.scala | 30 +++++++++---------- .../estimator/EstimatorGlobalVarTest.scala | 13 +++++++- .../com/wavesplatform/it/repl/ReplTest.scala | 3 +- .../RideReplBlockchainFunctionsSuite.scala | 3 +- .../predef/DAppVerifierRestrictionsTest.scala | 18 ++++++++++- .../wavesplatform/lang/v1/repl/package.scala | 2 +- 7 files changed, 50 insertions(+), 21 deletions(-) diff --git a/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/EstimatorContext.scala b/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/EstimatorContext.scala index d4b22846301..67a33cdc4ce 100644 --- a/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/EstimatorContext.scala +++ b/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/EstimatorContext.scala @@ -10,7 +10,7 @@ import shapeless.{Lens, lens} private[v3] case class EstimatorContext( funcs: Map[FunctionHeader, (Coeval[Long], Set[String])], usedRefs: Set[String] = Set(), - refsCosts: Map[String, Long] = Map(), + refsCosts: Map[String, EvalM[Long]] = Map(), globalFunctionsCosts: Map[String, Long] = Map(), // globalLetsCosts: Map[String, Long] = Map(), // only for globalDeclarationsMode globalLetEvals: Map[String, EvalM[Long]] = Map() // diff --git a/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/ScriptEstimatorV3.scala b/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/ScriptEstimatorV3.scala index d55ed46ff17..6d8dc073443 100644 --- a/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/ScriptEstimatorV3.scala +++ b/lang/shared/src/main/scala/com/wavesplatform/lang/v1/estimator/v3/ScriptEstimatorV3.scala @@ -1,8 +1,8 @@ package com.wavesplatform.lang.v1.estimator.v3 import cats.implicits.{toBifunctorOps, toFoldableOps, toTraverseOps} -import cats.{Id, Monad} import cats.syntax.functor.* +import cats.{Id, Monad} import com.wavesplatform.lang.v1.FunctionHeader import com.wavesplatform.lang.v1.FunctionHeader.User import com.wavesplatform.lang.v1.compiler.Terms.* @@ -100,14 +100,11 @@ case class ScriptEstimatorV3(fixOverflow: Boolean, overhead: Boolean, letFixes: } private def beforeNextExprEval(let: LET, eval: EvalM[Long]): EvalM[Unit] = - for { - cost <- local(eval) - _ <- update(ctx => - usedRefs - .modify(ctx)(_ - let.name) - .copy(refsCosts = ctx.refsCosts + (let.name -> cost)) - ) - } yield () + update(ctx => + usedRefs + .modify(ctx)(_ - let.name) + .copy(refsCosts = ctx.refsCosts + (let.name -> local(eval))) + ) private def afterNextExprEval(let: LET, startCtx: EstimatorContext): EvalM[Unit] = update(ctx => @@ -181,10 +178,11 @@ case class ScriptEstimatorV3(fixOverflow: Boolean, overhead: Boolean, letFixes: (argsCosts, _) <- withUsedRefs(args.traverse(evalHoldingFuncs(_, activeFuncArgs))) argsCostsSum <- argsCosts.foldM(0L)(sum) bodyCostV = bodyCost.value() - correctedBodyCost = - if (!overhead && !letFixes && bodyCostV == 0) 1 - else if (letFixes && bodyCostV == 0 && isBlankFunc(bodyUsedRefs, ctx.refsCosts)) 1 - else bodyCostV + correctedBodyCost <- + if (bodyCostV != 0) const(bodyCostV) + else if (overhead) zero + else if (!overhead && !letFixes) const(1L) + else isBlankFunc(bodyUsedRefs, ctx.refsCosts).map(if (_) 1L else 0L) result <- sum(argsCostsSum, correctedBodyCost) } yield result @@ -207,8 +205,10 @@ case class ScriptEstimatorV3(fixOverflow: Boolean, overhead: Boolean, letFixes: raiseError[Id, EstimatorContext, EstimationError, (Coeval[Long], Set[EstimationError])](s"function '$header' not found") ) - private def isBlankFunc(usedRefs: Set[String], refsCosts: Map[String, Long]): Boolean = - !usedRefs.exists(refsCosts.get(_).exists(_ > 0)) + private def isBlankFunc(usedRefs: Set[String], refsCosts: Map[String, EvalM[Long]]): EvalM[Boolean] = + usedRefs.toSeq + .existsM(refsCosts.get(_).existsM(_.map(_ > 0))) + .map(!_) private def evalHoldingFuncs( expr: EXPR, diff --git a/lang/tests/src/test/scala/com/wavesplatform/lang/estimator/EstimatorGlobalVarTest.scala b/lang/tests/src/test/scala/com/wavesplatform/lang/estimator/EstimatorGlobalVarTest.scala index 969ff042562..3ee9e5fa799 100644 --- a/lang/tests/src/test/scala/com/wavesplatform/lang/estimator/EstimatorGlobalVarTest.scala +++ b/lang/tests/src/test/scala/com/wavesplatform/lang/estimator/EstimatorGlobalVarTest.scala @@ -1,5 +1,5 @@ package com.wavesplatform.lang.estimator -import com.wavesplatform.lang.directives.values.{Expression, V6} +import com.wavesplatform.lang.directives.values.{Expression, V4, V6} import com.wavesplatform.lang.utils import com.wavesplatform.lang.utils.functionCosts import com.wavesplatform.lang.v1.FunctionHeader.{Native, User} @@ -130,4 +130,15 @@ class EstimatorGlobalVarTest extends ScriptEstimatorTestBase(ScriptEstimatorV3(f estimate(script) shouldBe Right(3) estimateFixed(script) shouldBe Right(3) } + + property("blank function call with/without overhead before let fixes") { + Seq(false, true).foreach(overhead => + ScriptEstimatorV3(fixOverflow = true, overhead, letFixes = false)( + lets, + functionCosts(V4), + compile("func f() = true\nf()") + ) shouldBe Right(1) + // with overhead 1 for "true", without — for blank function call + ) + } } diff --git a/node-it/src/test/scala/com/wavesplatform/it/repl/ReplTest.scala b/node-it/src/test/scala/com/wavesplatform/it/repl/ReplTest.scala index 859ce83679c..d0bf73c338f 100644 --- a/node-it/src/test/scala/com/wavesplatform/it/repl/ReplTest.scala +++ b/node-it/src/test/scala/com/wavesplatform/it/repl/ReplTest.scala @@ -178,9 +178,10 @@ class ReplTest extends BaseTransactionSuite with FailedTransactionSuiteLike[Stri | \\) | timestamp = \\d+ | vrf = base58'[$Base58Alphabet]+' - | height = $height | generationSignature = base58'[$Base58Alphabet]+' | generatorPublicKey = base58'[$Base58Alphabet]+${"" /*miner.publicKey*/}' + | height = $height + | rewards = \\[\\] |\\) """.trim.stripMargin diff --git a/node-it/src/test/scala/com/wavesplatform/it/sync/smartcontract/RideReplBlockchainFunctionsSuite.scala b/node-it/src/test/scala/com/wavesplatform/it/sync/smartcontract/RideReplBlockchainFunctionsSuite.scala index 2fa1f4a27e8..97c7d42e97c 100644 --- a/node-it/src/test/scala/com/wavesplatform/it/sync/smartcontract/RideReplBlockchainFunctionsSuite.scala +++ b/node-it/src/test/scala/com/wavesplatform/it/sync/smartcontract/RideReplBlockchainFunctionsSuite.scala @@ -182,9 +182,10 @@ class RideReplBlockchainFunctionsSuite extends BaseTransactionSuite { | ) | timestamp = ${bi.timestamp} | vrf = base58'${bi.vrf.get}' - | height = ${bi.height} | generationSignature = base58'${bi.generationSignature.get}' | generatorPublicKey = base58'${bi.generatorPublicKey}' + | height = ${bi.height} + | rewards = [] |) """.trim.stripMargin ) diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/smart/predef/DAppVerifierRestrictionsTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/smart/predef/DAppVerifierRestrictionsTest.scala index 56a2dbf5a54..c92df53f71d 100644 --- a/node/src/test/scala/com/wavesplatform/state/diffs/smart/predef/DAppVerifierRestrictionsTest.scala +++ b/node/src/test/scala/com/wavesplatform/state/diffs/smart/predef/DAppVerifierRestrictionsTest.scala @@ -14,6 +14,7 @@ import com.wavesplatform.lang.v1.estimator.v3.ScriptEstimatorV3 import com.wavesplatform.protobuf.dapp.DAppMeta import com.wavesplatform.test.* import com.wavesplatform.transaction.TxHelpers +import com.wavesplatform.transaction.TxHelpers.{defaultSigner, setScript} import org.scalatest.EitherValues class DAppVerifierRestrictionsTest extends PropSpec with WithDomain with EitherValues { @@ -196,7 +197,7 @@ class DAppVerifierRestrictionsTest extends PropSpec with WithDomain with EitherV def estimateScript(script: Script): Option[Long] = { val estimator = ScriptEstimatorV3.latest - Script.estimate(script, estimator, fixEstimateOfVerifier = false, useContractVerifierLimit = false).toOption + Script.estimate(script, estimator, fixEstimateOfVerifier = false, useContractVerifierLimit = false).toOption } val script1 = createScript( @@ -226,4 +227,19 @@ class DAppVerifierRestrictionsTest extends PropSpec with WithDomain with EitherV estimateScript(script4) shouldBe defined estimateScript(script5) shouldBe defined } + + property("An old buggy check that was in effect before RideV6 should not cause an error if a variable is not used in a @Callable") { + val script = TestCompiler(V5).compileContract( + """ + | func call() = { + | let asset = invoke(addressFromStringValue(""), "", nil, nil) + | nil + | } + | + | @Verifier(tx) + | func verify() = sigVerify(tx.bodyBytes, tx.proofs[0], tx.senderPublicKey) + """.stripMargin + ) + withDomain(RideV5, AddrWithBalance.enoughBalances(defaultSigner))(_.appendAndAssertSucceed(setScript(defaultSigner, script))) + } } diff --git a/repl/shared/src/main/scala/com/wavesplatform/lang/v1/repl/package.scala b/repl/shared/src/main/scala/com/wavesplatform/lang/v1/repl/package.scala index a2a8baa0989..54ba8356222 100644 --- a/repl/shared/src/main/scala/com/wavesplatform/lang/v1/repl/package.scala +++ b/repl/shared/src/main/scala/com/wavesplatform/lang/v1/repl/package.scala @@ -17,7 +17,7 @@ package object repl { val internalVarPrefixes: Set[Char] = Set('@', '$') val internalFuncPrefix: String = "_" - val version = V6 + val version = StdLibVersion.VersionDic.latest val directives: DirectiveSet = DirectiveSet(version, Account, DApp).explicitGet() val initialCtx: CTX[Environment] =