From beb188957e2910e7d0b72758ea38140082d58a4a Mon Sep 17 00:00:00 2001 From: Artyom Sayadyan Date: Mon, 9 Oct 2023 19:23:06 +0300 Subject: [PATCH 1/3] NODE-2531 Corrected validation order of sync payment balance --- .../state/diffs/TransactionDiffer.scala | 3 +- .../state/diffs/invoke/InvokeScriptDiff.scala | 58 +++++++++++-------- .../transaction/smart/WavesEnvironment.scala | 9 ++- .../smart/script/trace/CoevalR.scala | 2 - ...SyncInvokePaymentValidationOrderTest.scala | 47 +++++++++++++++ 5 files changed, 91 insertions(+), 28 deletions(-) create mode 100644 node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/TransactionDiffer.scala b/node/src/main/scala/com/wavesplatform/state/diffs/TransactionDiffer.scala index bb3a66cb1bd..25785141cb8 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/TransactionDiffer.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/TransactionDiffer.scala @@ -108,7 +108,8 @@ object TransactionDiffer { InvokeRejectError(fte.message, fte.log) case fte: FailedTransactionError if fte.isFailFree && blockchain.isFeatureActivated(RideV6) => ScriptExecutionError(fte.message, fte.log, fte.assetId) - case err => err + case err => + err } .leftMap(TransactionValidationError(_, tx)) } diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala index e5edbd8b435..6e1f94a218d 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala @@ -8,6 +8,7 @@ import cats.syntax.traverseFilter.* import com.wavesplatform.account.* import com.wavesplatform.common.state.ByteStr import com.wavesplatform.features.BlockchainFeatures +import com.wavesplatform.features.BlockchainFeatures.TransactionStateSnapshot import com.wavesplatform.features.EstimatorProvider.EstimatorBlockchainExt import com.wavesplatform.features.EvaluatorFixProvider.* import com.wavesplatform.features.FunctionCallPolicyProvider.* @@ -239,7 +240,11 @@ object InvokeScriptDiff { _ = invocationRoot.setLog(log) spentComplexity = remainingComplexity - scriptResult.unusedComplexity.max(0) - _ <- validateIntermediateBalances(blockchain, resultSnapshot, spentComplexity, log) + _ <- + if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + traced(Right(())) + else + validateIntermediateBalances(blockchain, resultSnapshot, spentComplexity, log) doProcessActions = (actions: List[CallableAction], unusedComplexity: Int) => { val storingComplexity = complexityAfterPayments - unusedComplexity @@ -347,7 +352,11 @@ object InvokeScriptDiff { resultSnapshot <- traced( (resultSnapshot.setScriptsComplexity(0) |+| actionsSnapshot.addScriptsComplexity(paymentsComplexity)).asRight ) - _ <- validateIntermediateBalances(blockchain, resultSnapshot, resultSnapshot.scriptsComplexity, log) + _ <- + if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + traced(Right(())) + else + validateIntermediateBalances(blockchain, resultSnapshot, resultSnapshot.scriptsComplexity, log) _ = invocationRoot.setResult(scriptResult) } yield ( resultSnapshot, @@ -419,30 +428,31 @@ object InvokeScriptDiff { ) } - private def validateIntermediateBalances(blockchain: Blockchain, snapshot: StateSnapshot, spentComplexity: Long, log: Log[Id]) = traced( - if (blockchain.isFeatureActivated(BlockchainFeatures.RideV6)) { - BalanceDiffValidation(blockchain)(snapshot) - .leftMap { be => FailedTransactionError.dAppExecution(be.toString, spentComplexity, log) } - } else if (blockchain.height >= blockchain.settings.functionalitySettings.enforceTransferValidationAfter) { - // reject transaction if any balance is negative - snapshot.balances.view - .flatMap { - case ((address, asset), balance) if balance < 0 => Some(address -> asset) - case _ => None - } - .headOption - .fold[Either[ValidationError, Unit]](Right(())) { case (address, asset) => - val msg = asset match { - case Waves => - s"$address: Negative waves balance: old = ${blockchain.balance(address)}, new = ${snapshot.balances((address, Waves))}" - case ia: IssuedAsset => - s"$address: Negative asset $ia balance: old = ${blockchain.balance(address, ia)}, new = ${snapshot.balances((address, ia))}" + def validateIntermediateBalances(blockchain: Blockchain, snapshot: StateSnapshot, spentComplexity: Long, log: Log[Id]): CoevalR[Any] = + traced( + if (blockchain.isFeatureActivated(BlockchainFeatures.RideV6)) { + BalanceDiffValidation(blockchain)(snapshot) + .leftMap { be => FailedTransactionError.dAppExecution(be.toString, spentComplexity, log) } + } else if (blockchain.height >= blockchain.settings.functionalitySettings.enforceTransferValidationAfter) { + // reject transaction if any balance is negative + snapshot.balances.view + .flatMap { + case ((address, asset), balance) if balance < 0 => Some(address -> asset) + case _ => None + } + .headOption + .fold[Either[ValidationError, Unit]](Right(())) { case (address, asset) => + val msg = asset match { + case Waves => + s"$address: Negative waves balance: old = ${blockchain.balance(address)}, new = ${snapshot.balances((address, Waves))}" + case ia: IssuedAsset => + s"$address: Negative asset $ia balance: old = ${blockchain.balance(address, ia)}, new = ${snapshot.balances((address, ia))}" + } + Left(FailOrRejectError(msg)) } - Left(FailOrRejectError(msg)) - } - } else Right(()) - ) + } else Right(()) + ) private def ensurePaymentsAreNotNegative(blockchain: Blockchain, tx: InvokeScript, invoker: Address, dAppAddress: Address) = traced { tx.payments.collectFirst { diff --git a/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala b/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala index 1aa4c7159b2..ef5b3b2a027 100644 --- a/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala +++ b/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala @@ -9,8 +9,8 @@ import com.wavesplatform.common.state.ByteStr import com.wavesplatform.common.utils.EitherExt2 import com.wavesplatform.consensus.{FairPoSCalculator, PoSCalculator} import com.wavesplatform.features.BlockchainFeatures +import com.wavesplatform.features.BlockchainFeatures.TransactionStateSnapshot import com.wavesplatform.features.MultiPaymentPolicyProvider.* -import com.wavesplatform.lang.{Global, ValidationError} import com.wavesplatform.lang.directives.DirectiveSet import com.wavesplatform.lang.directives.values.StdLibVersion import com.wavesplatform.lang.script.Script @@ -20,8 +20,10 @@ import com.wavesplatform.lang.v1.evaluator.{Log, ScriptResult} import com.wavesplatform.lang.v1.traits.* import com.wavesplatform.lang.v1.traits.domain.* import com.wavesplatform.lang.v1.traits.domain.Recipient.* +import com.wavesplatform.lang.{Global, ValidationError} import com.wavesplatform.state.* import com.wavesplatform.state.BlockRewardCalculator.CurrentBlockRewardPart +import com.wavesplatform.state.diffs.invoke.InvokeScriptDiff.validateIntermediateBalances import com.wavesplatform.state.diffs.invoke.{InvokeScript, InvokeScriptDiff, InvokeScriptTransactionLike} import com.wavesplatform.state.reader.SnapshotBlockchain import com.wavesplatform.transaction.Asset.* @@ -446,6 +448,11 @@ class DAppEnvironment( if (reentrant) calledAddresses else calledAddresses + invoke.sender.toAddress, invocationTracker )(invoke) + _ <- + if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + validateIntermediateBalances(blockchain, snapshot, totalComplexityLimit - availableComplexity, Nil) + else + traced(Right(())) fixedSnapshot = snapshot .setScriptResults(Map(txId -> InvokeScriptResult(invokes = Seq(invocation.copy(stateChanges = snapshot.scriptResults(txId)))))) } yield { diff --git a/node/src/main/scala/com/wavesplatform/transaction/smart/script/trace/CoevalR.scala b/node/src/main/scala/com/wavesplatform/transaction/smart/script/trace/CoevalR.scala index 1ce52abee1e..3ae8623429c 100644 --- a/node/src/main/scala/com/wavesplatform/transaction/smart/script/trace/CoevalR.scala +++ b/node/src/main/scala/com/wavesplatform/transaction/smart/script/trace/CoevalR.scala @@ -1,6 +1,4 @@ package com.wavesplatform.transaction.smart.script.trace -import scala.util.Right - import com.wavesplatform.lang.ValidationError import monix.eval.Coeval diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala new file mode 100644 index 00000000000..37683f81b79 --- /dev/null +++ b/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala @@ -0,0 +1,47 @@ +package com.wavesplatform.state.diffs.ci.sync + +import com.wavesplatform.db.WithDomain +import com.wavesplatform.db.WithState.AddrWithBalance +import com.wavesplatform.lang.directives.values.V7 +import com.wavesplatform.lang.v1.compiler.Terms.CONST_BOOLEAN +import com.wavesplatform.lang.v1.compiler.TestCompiler +import com.wavesplatform.test.DomainPresets.{BlockRewardDistribution, TransactionStateSnapshot} +import com.wavesplatform.test.{PropSpec, produce} +import com.wavesplatform.transaction.Asset.IssuedAsset +import com.wavesplatform.transaction.TxHelpers.* + +class SyncInvokePaymentValidationOrderTest extends PropSpec with WithDomain { + private val issueTx = issue() + private val asset = IssuedAsset(issueTx.id()) + private val dApp = TestCompiler(V7).compileContract( + s""" + | @Callable(i) + | func f1(fail: Boolean) = { + | strict r = Address(base58'$defaultAddress').invoke("f2", [fail], [AttachedPayment(base58'$asset', 123)]) + | [] + | } + | + | @Callable(i) + | func f2(fail: Boolean) = { + | strict c = if (fail) then ${(1 to 10).map(_ => "sigVerify(base58'', base58'', base58'')").mkString(" || ")} else 0 + | [] + | } + """.stripMargin + ) + + property("sync invoke payment should be processed before calling dApp if light node isn't activated") { + withDomain(BlockRewardDistribution, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => + d.appendBlock(setScript(defaultSigner, dApp), setScript(secondSigner, dApp), issueTx) + d.appendAndAssertFailed(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true))), "negative asset balance") + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false)))) should produce("negative asset balance") + } + } + + property("sync invoke payment should be processed before calling dApp if light node is activated") { + withDomain(TransactionStateSnapshot, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => + d.appendBlock(setScript(defaultSigner, dApp), setScript(secondSigner, dApp), issueTx) + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true)))) should produce("negative asset balance") + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false)))) should produce("negative asset balance") + } + } +} From 123e8f3d120ca4027ca6ba648949161cabae6b73 Mon Sep 17 00:00:00 2001 From: Artyom Sayadyan Date: Mon, 9 Oct 2023 19:54:53 +0300 Subject: [PATCH 2/3] Corrected imports --- .../wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala | 6 +++--- .../wavesplatform/transaction/smart/WavesEnvironment.scala | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala index 6e1f94a218d..5f9c13c1615 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeScriptDiff.scala @@ -8,7 +8,7 @@ import cats.syntax.traverseFilter.* import com.wavesplatform.account.* import com.wavesplatform.common.state.ByteStr import com.wavesplatform.features.BlockchainFeatures -import com.wavesplatform.features.BlockchainFeatures.TransactionStateSnapshot +import com.wavesplatform.features.BlockchainFeatures.LightNode import com.wavesplatform.features.EstimatorProvider.EstimatorBlockchainExt import com.wavesplatform.features.EvaluatorFixProvider.* import com.wavesplatform.features.FunctionCallPolicyProvider.* @@ -241,7 +241,7 @@ object InvokeScriptDiff { spentComplexity = remainingComplexity - scriptResult.unusedComplexity.max(0) _ <- - if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + if (blockchain.isFeatureActivated(LightNode)) traced(Right(())) else validateIntermediateBalances(blockchain, resultSnapshot, spentComplexity, log) @@ -353,7 +353,7 @@ object InvokeScriptDiff { (resultSnapshot.setScriptsComplexity(0) |+| actionsSnapshot.addScriptsComplexity(paymentsComplexity)).asRight ) _ <- - if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + if (blockchain.isFeatureActivated(LightNode)) traced(Right(())) else validateIntermediateBalances(blockchain, resultSnapshot, resultSnapshot.scriptsComplexity, log) diff --git a/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala b/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala index ef5b3b2a027..accb6e6e7ea 100644 --- a/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala +++ b/node/src/main/scala/com/wavesplatform/transaction/smart/WavesEnvironment.scala @@ -9,7 +9,7 @@ import com.wavesplatform.common.state.ByteStr import com.wavesplatform.common.utils.EitherExt2 import com.wavesplatform.consensus.{FairPoSCalculator, PoSCalculator} import com.wavesplatform.features.BlockchainFeatures -import com.wavesplatform.features.BlockchainFeatures.TransactionStateSnapshot +import com.wavesplatform.features.BlockchainFeatures.LightNode import com.wavesplatform.features.MultiPaymentPolicyProvider.* import com.wavesplatform.lang.directives.DirectiveSet import com.wavesplatform.lang.directives.values.StdLibVersion @@ -449,7 +449,7 @@ class DAppEnvironment( invocationTracker )(invoke) _ <- - if (blockchain.isFeatureActivated(TransactionStateSnapshot)) + if (blockchain.isFeatureActivated(LightNode)) validateIntermediateBalances(blockchain, snapshot, totalComplexityLimit - availableComplexity, Nil) else traced(Right(())) From f2cd578449abd873904e7f976d31fc475086f66d Mon Sep 17 00:00:00 2001 From: Artyom Sayadyan Date: Tue, 10 Oct 2023 11:39:29 +0300 Subject: [PATCH 3/3] Better tests --- ...SyncInvokePaymentValidationOrderTest.scala | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala index 37683f81b79..ad3f88c1ee5 100644 --- a/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala +++ b/node/src/test/scala/com/wavesplatform/state/diffs/ci/sync/SyncInvokePaymentValidationOrderTest.scala @@ -16,32 +16,56 @@ class SyncInvokePaymentValidationOrderTest extends PropSpec with WithDomain { private val dApp = TestCompiler(V7).compileContract( s""" | @Callable(i) - | func f1(fail: Boolean) = { - | strict r = Address(base58'$defaultAddress').invoke("f2", [fail], [AttachedPayment(base58'$asset', 123)]) + | func f1(bigComplexity: Boolean, error: Boolean) = { + | strict r = Address(base58'$defaultAddress').invoke("f2", [bigComplexity, error], [AttachedPayment(base58'$asset', 123)]) | [] | } | | @Callable(i) - | func f2(fail: Boolean) = { - | strict c = if (fail) then ${(1 to 10).map(_ => "sigVerify(base58'', base58'', base58'')").mkString(" || ")} else 0 + | func f2(bigComplexity: Boolean, error: Boolean) = { + | strict c = if (bigComplexity) then ${(1 to 6).map(_ => "sigVerify(base58'', base58'', base58'')").mkString(" || ")} else 0 + | strict e = if (error) then throw("custom error") else 0 | [] | } """.stripMargin ) - property("sync invoke payment should be processed before calling dApp if light node isn't activated") { + property("sync invoke payment should be validated after calling dApp if light node isn't activated") { withDomain(BlockRewardDistribution, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => d.appendBlock(setScript(defaultSigner, dApp), setScript(secondSigner, dApp), issueTx) - d.appendAndAssertFailed(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true))), "negative asset balance") - d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false)))) should produce("negative asset balance") + d.appendAndAssertFailed( + invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true), CONST_BOOLEAN(false))), + "negative asset balance" + ) + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false), CONST_BOOLEAN(false)))) should produce( + "negative asset balance" + ) } } - property("sync invoke payment should be processed before calling dApp if light node is activated") { + property("sync invoke payment should be validated before calling dApp if light node is activated") { withDomain(TransactionStateSnapshot, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => d.appendBlock(setScript(defaultSigner, dApp), setScript(secondSigner, dApp), issueTx) - d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true)))) should produce("negative asset balance") - d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false)))) should produce("negative asset balance") + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true), CONST_BOOLEAN(false)))) should produce( + "negative asset balance" + ) + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false), CONST_BOOLEAN(false)))) should produce( + "negative asset balance" + ) + } + } + + property("sync invoke should be correctly rejected and failed on enough balance and RIDE error if light node is activated") { + withDomain(TransactionStateSnapshot, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => + d.appendBlock(setScript(defaultSigner, dApp), setScript(secondSigner, dApp), issueTx) + d.appendBlock(transfer(asset = asset)) + d.appendBlockE(invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(false), CONST_BOOLEAN(true)))) should produce( + "custom error" + ) + d.appendAndAssertFailed( + invoke(invoker = secondSigner, func = Some("f1"), args = Seq(CONST_BOOLEAN(true), CONST_BOOLEAN(true))), + "custom error" + ) } } }