From 26f5194fb6cd254a9c9db0164c0abee49a84488a Mon Sep 17 00:00:00 2001 From: Artyom Sayadyan Date: Mon, 27 Nov 2023 09:46:28 +0300 Subject: [PATCH] NODE-2503 Better transaction asset payment errors (#3890) --- .../state/diffs/CommonValidation.scala | 16 ++- .../state/diffs/EthereumTransactionDiff.scala | 3 +- .../diffs/invoke/InvokeDiffsCommon.scala | 14 ++- .../diffs/ci/InvokeAssetChecksTest.scala | 2 +- .../state/diffs/ci/InvokeValidationTest.scala | 12 -- .../diffs/ci/TransactionAssetChecksTest.scala | 113 ++++++++++++++++++ 6 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 node/src/test/scala/com/wavesplatform/state/diffs/ci/TransactionAssetChecksTest.scala diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala b/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala index d4d58d7adb..66cc7a4043 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala @@ -13,7 +13,7 @@ import com.wavesplatform.lang.script.v1.ExprScript import com.wavesplatform.lang.script.{ContractScript, Script} import com.wavesplatform.settings.FunctionalitySettings import com.wavesplatform.state.* -import com.wavesplatform.transaction.* +import com.wavesplatform.state.diffs.invoke.InvokeDiffsCommon import com.wavesplatform.transaction.Asset.{IssuedAsset, Waves} import com.wavesplatform.transaction.TxValidationError.* import com.wavesplatform.transaction.assets.* @@ -22,6 +22,7 @@ import com.wavesplatform.transaction.lease.* import com.wavesplatform.transaction.smart.InvokeScriptTransaction.Payment import com.wavesplatform.transaction.smart.{InvokeExpressionTransaction, InvokeScriptTransaction, SetScriptTransaction} import com.wavesplatform.transaction.transfer.* +import com.wavesplatform.transaction.{Asset, *} import scala.util.{Left, Right} @@ -36,21 +37,25 @@ object CommonValidation { feeAmount: Long, allowFeeOverdraft: Boolean = false ): Either[ValidationError, T] = { - val amountDiff = assetId match { + val amountPortfolio = assetId match { case aid @ IssuedAsset(_) => Portfolio.build(aid -> -amount) case Waves => Portfolio(-amount) } - val feeDiff = feeAssetId match { + val feePortfolio = feeAssetId match { case aid @ IssuedAsset(_) => Portfolio.build(aid -> -feeAmount) case Waves => Portfolio(-feeAmount) } val checkedTx = for { - spendings <- amountDiff.combine(feeDiff) + _ <- assetId match { + case IssuedAsset(id) => InvokeDiffsCommon.checkAsset(blockchain, id) + case Waves => Right(()) + } + spendings <- amountPortfolio.combine(feePortfolio) oldWavesBalance = blockchain.balance(sender, Waves) newWavesBalance <- safeSum(oldWavesBalance, spendings.balance, "Spendings") - feeUncheckedBalance <- safeSum(oldWavesBalance, amountDiff.balance, "Transfer amount") + feeUncheckedBalance <- safeSum(oldWavesBalance, amountPortfolio.balance, "Transfer amount") overdraftFilter = allowFeeOverdraft && feeUncheckedBalance >= 0 _ <- Either.cond( @@ -95,6 +100,7 @@ object CommonValidation { for { address <- blockchain.resolveAlias(citx.dApp) + _ <- InvokeDiffsCommon.checkPayments(blockchain, citx.payments) allowFeeOverdraft = blockchain.accountScript(address) match { case Some(AccountScriptInfo(_, ContractScriptImpl(version, _), _, _)) if version >= V4 && blockchain.useCorrectPaymentCheck => true case _ => false diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/EthereumTransactionDiff.scala b/node/src/main/scala/com/wavesplatform/state/diffs/EthereumTransactionDiff.scala index 382081b76d..2d3f5b39cb 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/EthereumTransactionDiff.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/EthereumTransactionDiff.scala @@ -8,7 +8,7 @@ import com.wavesplatform.features.BlockchainFeatures import com.wavesplatform.lang.ValidationError import com.wavesplatform.lang.v1.serialization.SerdeV1 import com.wavesplatform.protobuf.transaction.{PBAmounts, PBRecipients} -import com.wavesplatform.state.diffs.invoke.InvokeScriptTransactionDiff +import com.wavesplatform.state.diffs.invoke.{InvokeDiffsCommon, InvokeScriptTransactionDiff} import com.wavesplatform.state.{Blockchain, StateSnapshot} import com.wavesplatform.transaction.EthereumTransaction import com.wavesplatform.transaction.TxValidationError.GenericError @@ -78,6 +78,7 @@ object EthereumTransactionDiff { for { _ <- checkLeadingZeros(tx, blockchain) invocation <- TracedResult(ei.toInvokeScriptLike(tx, blockchain)) + _ <- TracedResult(InvokeDiffsCommon.checkPayments(blockchain, invocation.payments)) snapshot <- InvokeScriptTransactionDiff(blockchain, currentBlockTs, limitedExecution, enableExecutionLog)(invocation) resultSnapshot <- TransactionDiffer.assetsVerifierDiff( blockchain, diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeDiffsCommon.scala b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeDiffsCommon.scala index e154bb12c4..b00d78891e 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeDiffsCommon.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/invoke/InvokeDiffsCommon.scala @@ -30,6 +30,7 @@ import com.wavesplatform.transaction.TxValidationError.* import com.wavesplatform.transaction.assets.IssueTransaction import com.wavesplatform.transaction.smart.* import com.wavesplatform.transaction.smart.DAppEnvironment.ActionLimits +import com.wavesplatform.transaction.smart.InvokeScriptTransaction.Payment import com.wavesplatform.transaction.smart.script.ScriptRunner import com.wavesplatform.transaction.smart.script.ScriptRunner.TxOrd import com.wavesplatform.transaction.smart.script.trace.AssetVerifierTrace.AssetContext @@ -345,6 +346,15 @@ object InvokeDiffsCommon { ) } + def checkPayments(blockchain: Blockchain, payments: Seq[Payment]): Either[GenericError, Unit] = + payments + .collectFirstSome { + case Payment(_, IssuedAsset(id)) => InvokeDiffsCommon.checkAsset(blockchain, id).swap.toOption + case Payment(_, Waves) => None + } + .map(GenericError(_)) + .toLeft(()) + def checkAsset(blockchain: Blockchain, assetId: ByteStr): Either[String, Unit] = if (blockchain.isFeatureActivated(BlockchainFeatures.SynchronousCalls)) if (assetId.size != AssetIdLength) @@ -464,8 +474,8 @@ object InvokeDiffsCommon { if (remainingLimit < Int.MaxValue) remainingLimit - currentSnapshot.scriptsComplexity.toInt else remainingLimit - val blockchain = SnapshotBlockchain(sblockchain, currentSnapshot) - val actionSender = Recipient.Address(ByteStr(dAppAddress.bytes)) + val blockchain = SnapshotBlockchain(sblockchain, currentSnapshot) + val actionSender = Recipient.Address(ByteStr(dAppAddress.bytes)) def applyTransfer(transfer: AssetTransfer, pk: PublicKey): TracedResult[ValidationError, StateSnapshot] = { val AssetTransfer(addressRepr, recipient, amount, asset) = transfer diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeAssetChecksTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeAssetChecksTest.scala index c8ecf6b0bd..0deaa03630 100644 --- a/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeAssetChecksTest.scala +++ b/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeAssetChecksTest.scala @@ -26,7 +26,7 @@ class InvokeAssetChecksTest extends PropSpec with Inside with WithState with DBC private val lengthError = s"Transfer error: invalid asset ID '$invalidLengthAsset' length = 4 bytes, must be 32" private val nonExistentError = s"Transfer error: asset '$nonExistentAsset' is not found on the blockchain" - property("invoke asset checks") { + property("invoke transfer checks") { val dApp = TestCompiler(V4).compileContract( s""" |@Callable(i) diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeValidationTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeValidationTest.scala index 7efbb87d69..2fd04239c2 100644 --- a/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeValidationTest.scala +++ b/node/src/test/scala/com/wavesplatform/state/diffs/ci/InvokeValidationTest.scala @@ -1,7 +1,6 @@ package com.wavesplatform.state.diffs.ci import com.wavesplatform.TestValues.invokeFee import com.wavesplatform.account.Alias -import com.wavesplatform.common.state.ByteStr import com.wavesplatform.common.utils.EitherExt2 import com.wavesplatform.db.WithDomain import com.wavesplatform.db.WithState.AddrWithBalance @@ -125,15 +124,4 @@ class InvokeValidationTest extends PropSpec with WithDomain { PBTransactions.toPBInvokeScriptData(txV2.dApp, txV2.funcCallOpt, txV2.payments).toByteArray.length shouldBe 5120 (the[Exception] thrownBy tooBigTxV2).getMessage shouldBe "GenericError(InvokeScriptTransaction bytes length = 5129 exceeds limit = 5120)" } - - property("unexisting payment asset") { - withDomain(RideV5) { d => - val asset = IssuedAsset(ByteStr.fromBytes(1, 2, 3)) - d.appendBlockE(invoke(defaultAddress, payments = Seq(Payment(1, asset)))) should produce( - "Attempt to transfer unavailable funds: " + - s"Transaction application leads to negative asset '$asset' balance to (at least) temporary negative state, " + - "current balance is 0, spends equals -1, result is -1" - ) - } - } } diff --git a/node/src/test/scala/com/wavesplatform/state/diffs/ci/TransactionAssetChecksTest.scala b/node/src/test/scala/com/wavesplatform/state/diffs/ci/TransactionAssetChecksTest.scala new file mode 100644 index 0000000000..81317a31b7 --- /dev/null +++ b/node/src/test/scala/com/wavesplatform/state/diffs/ci/TransactionAssetChecksTest.scala @@ -0,0 +1,113 @@ +package com.wavesplatform.state.diffs.ci + +import com.wavesplatform.common.state.ByteStr +import com.wavesplatform.db.WithDomain +import com.wavesplatform.db.WithState.AddrWithBalance +import com.wavesplatform.lang.directives.values.V8 +import com.wavesplatform.lang.v1.compiler.TestCompiler +import com.wavesplatform.test.DomainPresets.* +import com.wavesplatform.test.{PropSpec, produce} +import com.wavesplatform.transaction.Asset.IssuedAsset +import com.wavesplatform.transaction.EthTxGenerator.{generateEthInvoke, generateEthTransfer} +import com.wavesplatform.transaction.TxHelpers.* +import com.wavesplatform.transaction.smart.InvokeScriptTransaction.Payment +import com.wavesplatform.transaction.utils.EthConverters.EthereumKeyPairExt + +import scala.util.Try + +class TransactionAssetChecksTest extends PropSpec with WithDomain { + private val dApp = TestCompiler(V8).compileContract( + """ + | @Callable(i) + | func default() = [] + """.stripMargin + ) + private val issueTx = issue(secondSigner) + private val asset = IssuedAsset(issueTx.id()) + + property("invoke script transaction") { + withDomain(TransactionStateSnapshot, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => + d.appendBlock(setScript(secondSigner, dApp), issueTx) + d.appendBlockE(invoke(secondAddress, payments = Seq(Payment(1, IssuedAsset(ByteStr.fill(31)(1)))))) should produce( + "invalid asset ID 'tVojvhToWjQ8Xvo4UPx2Xz9eRy7auyYMmZBjc2XfN' length = 31 bytes, must be 32" + ) + d.appendBlockE(invoke(secondAddress, payments = Seq(Payment(1, IssuedAsset(ByteStr.fill(33)(1)))))) should produce( + "invalid asset ID 'JJEfe6DcPM2ziB2vfUWDV6aHVerXRGkv3TcyvJUNGHZz' length = 33 bytes, must be 32" + ) + d.appendBlockE(invoke(secondAddress, payments = Seq(Payment(1, IssuedAsset(ByteStr.fill(32)(1)))))) should produce( + "asset '4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi' is not found on the blockchain" + ) + val invokeWithIssued = invoke(secondAddress, payments = Seq(Payment(1, asset))) + d.appendBlockE(invokeWithIssued) should produce(s"leads to negative asset '$asset' balance") + d.appendBlock(transfer(secondSigner, defaultAddress, asset = asset)) + d.appendAndAssertSucceed(invokeWithIssued) + } + } + + property("ethereum invoke script transaction") { + withDomain( + TransactionStateSnapshot, + AddrWithBalance.enoughBalances(defaultSigner, secondSigner) ++ Seq( + AddrWithBalance(defaultSigner.toEthWavesAddress), + AddrWithBalance(secondSigner.toEthWavesAddress) + ) + ) { d => + d.appendBlock(setScript(secondSigner, dApp), issueTx, transfer(secondSigner, defaultSigner.toEthWavesAddress, asset = asset)) + Try( + generateEthInvoke(defaultEthSigner, secondAddress, "default", Nil, Seq(Payment(1, IssuedAsset(ByteStr.fill(31)(1))))) + ).toEither should produce("InvocationTargetException") + Try( + generateEthInvoke(defaultEthSigner, secondAddress, "default", Nil, Seq(Payment(1, IssuedAsset(ByteStr.fill(33)(1))))) + ).toEither should produce("InvocationTargetException") + d.appendBlockE( + generateEthInvoke(defaultEthSigner, secondAddress, "default", Nil, Seq(Payment(1, IssuedAsset(ByteStr.fill(32)(1))))) + ) should produce( + "asset '4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi' is not found on the blockchain" + ) + val invokeWithIssued = generateEthInvoke(secondSigner.toEthKeyPair, secondAddress, "default", Nil, payments = Seq(Payment(1, asset))) + d.appendBlockE(invokeWithIssued) should produce("negative asset balance") + d.appendBlock(transfer(secondSigner, secondSigner.toEthWavesAddress, asset = asset)) + d.appendAndAssertSucceed(invokeWithIssued) + } + } + + property("transfer transaction") { + withDomain(TransactionStateSnapshot, AddrWithBalance.enoughBalances(defaultSigner, secondSigner)) { d => + d.appendBlock(setScript(secondSigner, dApp), issueTx) + d.appendBlockE(transfer(asset = IssuedAsset(ByteStr.fill(31)(1)))) should produce( + "invalid asset ID 'tVojvhToWjQ8Xvo4UPx2Xz9eRy7auyYMmZBjc2XfN' length = 31 bytes, must be 32" + ) + d.appendBlockE(transfer(asset = IssuedAsset(ByteStr.fill(33)(1)))) should produce( + "invalid asset ID 'JJEfe6DcPM2ziB2vfUWDV6aHVerXRGkv3TcyvJUNGHZz' length = 33 bytes, must be 32" + ) + d.appendBlockE(transfer(asset = IssuedAsset(ByteStr.fill(32)(1)))) should produce( + "asset '4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi' is not found on the blockchain" + ) + val transferIssued = transfer(asset = asset) + d.appendBlockE(transferIssued) should produce(s"leads to negative asset '$asset' balance") + d.appendBlock(transfer(secondSigner, defaultAddress, asset = asset)) + d.appendAndAssertSucceed(transferIssued) + } + } + + property("ethereum transfer transaction") { + withDomain( + TransactionStateSnapshot, + AddrWithBalance.enoughBalances(defaultSigner, secondSigner) ++ Seq( + AddrWithBalance(defaultSigner.toEthWavesAddress), + AddrWithBalance(secondSigner.toEthWavesAddress) + ) + ) { d => + d.appendBlock(setScript(secondSigner, dApp), issueTx, transfer(secondSigner, defaultSigner.toEthWavesAddress, asset = asset)) + (31 to 33).foreach(i => + d.appendBlockE(generateEthTransfer(defaultEthSigner, secondAddress, 1, IssuedAsset(ByteStr.fill(i)(1)))) should produce( + "Can't resolve ERC20 address" + ) + ) + val transferIssued = generateEthTransfer(secondSigner.toEthKeyPair, secondAddress, 1, asset) + d.appendBlockE(transferIssued) should produce(s"negative asset balance") + d.appendBlock(transfer(secondSigner, secondSigner.toEthWavesAddress, asset = asset)) + d.appendAndAssertSucceed(transferIssued) + } + } +}