From 2695b5a3267f2854d3c84c3568d580f167304a97 Mon Sep 17 00:00:00 2001 From: Karasiq Date: Mon, 20 Sep 2021 19:53:00 +0300 Subject: [PATCH] Disallow negative miner balance and base target --- .../consensus/PoSCalculator.scala | 13 +++++--- .../state/DiffToStateApplier.scala | 6 ++-- .../state/diffs/BlockDiffer.scala | 9 ++++-- .../state/diffs/CommonValidation.scala | 32 +++++++++++++++---- .../consensus/FairPoSCalculatorTest.scala | 13 +++++--- .../com/wavesplatform/db/WithState.scala | 14 ++++---- .../com/wavesplatform/history/Domain.scala | 11 +++++++ .../mining/MinerBalanceOverflowTest.scala | 15 +++++++++ 8 files changed, 86 insertions(+), 27 deletions(-) create mode 100644 node/src/test/scala/com/wavesplatform/mining/MinerBalanceOverflowTest.scala diff --git a/node/src/main/scala/com/wavesplatform/consensus/PoSCalculator.scala b/node/src/main/scala/com/wavesplatform/consensus/PoSCalculator.scala index 2b322134a54..342d9759ac3 100644 --- a/node/src/main/scala/com/wavesplatform/consensus/PoSCalculator.scala +++ b/node/src/main/scala/com/wavesplatform/consensus/PoSCalculator.scala @@ -1,5 +1,7 @@ package com.wavesplatform.consensus +import scala.util.Try + import com.wavesplatform.account.{PrivateKey, PublicKey} import com.wavesplatform.common.state.ByteStr import com.wavesplatform.consensus.PoSCalculator.HitSize @@ -20,8 +22,8 @@ trait PoSCalculator { } object PoSCalculator { - private[consensus] val HitSize: Int = 8 - val MinBaseTarget: Long = 9 + private[consensus] val HitSize: Int = 8 + val MinBaseTarget: Long = 9 def generationSignature(signature: ByteStr, publicKey: PublicKey): Array[Byte] = { val s = new Array[Byte](crypto.DigestLength * 2) @@ -121,11 +123,12 @@ case class FairPoSCalculator(minBlockTime: Int, delayDelta: Int) extends PoSCalc maybeGreatGrandParentTimestamp match { case None => prevBaseTarget + case Some(ts) => - val avg = (timestamp - ts) / 3 / 1000 + val avg = (timestamp - ts) / 3 / 1000 val prevBaseTarget1pct = (prevBaseTarget / 100) max 1 - if (avg > maxDelay) prevBaseTarget + prevBaseTarget1pct - else if (avg < minDelay) (prevBaseTarget - prevBaseTarget1pct) max 1 + if (avg > maxDelay) Try(math.addExact(prevBaseTarget, prevBaseTarget1pct)).getOrElse(Long.MaxValue) + else if (avg < minDelay) math.subtractExact(prevBaseTarget, prevBaseTarget1pct) max 1 else prevBaseTarget } } diff --git a/node/src/main/scala/com/wavesplatform/state/DiffToStateApplier.scala b/node/src/main/scala/com/wavesplatform/state/DiffToStateApplier.scala index c4056a47363..c403319b558 100644 --- a/node/src/main/scala/com/wavesplatform/state/DiffToStateApplier.scala +++ b/node/src/main/scala/com/wavesplatform/state/DiffToStateApplier.scala @@ -25,12 +25,12 @@ object DiffToStateApplier { val bs = Map.newBuilder[Asset, Long] if (portfolioDiff.balance != 0) { - bs += Waves -> (blockchain.balance(address, Waves) + portfolioDiff.balance) + bs += Waves -> math.addExact(blockchain.balance(address, Waves), portfolioDiff.balance).ensuring(_ > 0) } portfolioDiff.assets.collect { - case (asset, balanceDiff) if balanceDiff != 0 => - bs += asset -> (blockchain.balance(address, asset) + balanceDiff) + case (asset, delta) if delta != 0 => + bs += asset -> math.addExact(blockchain.balance(address, asset), delta).ensuring(_ > 0) } balances += address -> bs.result() diff --git a/node/src/main/scala/com/wavesplatform/state/diffs/BlockDiffer.scala b/node/src/main/scala/com/wavesplatform/state/diffs/BlockDiffer.scala index d6c53ba9275..94846ac86ae 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/BlockDiffer.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/BlockDiffer.scala @@ -13,7 +13,7 @@ import com.wavesplatform.state.patch._ import com.wavesplatform.state.reader.CompositeBlockchain import com.wavesplatform.transaction.{Asset, Transaction} import com.wavesplatform.transaction.Asset.{IssuedAsset, Waves} -import com.wavesplatform.transaction.TxValidationError.{ActivationError, _} +import com.wavesplatform.transaction.TxValidationError._ import com.wavesplatform.transaction.smart.script.trace.TracedResult import com.wavesplatform.utils.ScorexLogging @@ -156,7 +156,7 @@ object BlockDiffer extends ScorexLogging { patch.lift(CompositeBlockchain(blockchain, prevDiff)).fold(prevDiff)(prevDiff |+| _) } - txs + val result = txs .foldLeft(TracedResult(Result(initDiffWithPatches, 0L, 0L, initConstraint, DetailedDiff(initDiffWithPatches, Nil)).asRight[ValidationError])) { case (acc @ TracedResult(Left(_), _, _), _) => acc case (TracedResult(Right(Result(currDiff, carryFee, currTotalFee, currConstraint, DetailedDiff(parentDiff, txDiffs))), _, _), tx) => @@ -193,5 +193,10 @@ object BlockDiffer extends ScorexLogging { } } } + + for { + result <- result + _ <- CommonValidation.disallowNegativeBalances(blockchain, result.diff) + } yield result } } 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 8fe1f55c464..77f6f9641dd 100644 --- a/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala +++ b/node/src/main/scala/com/wavesplatform/state/diffs/CommonValidation.scala @@ -1,30 +1,32 @@ package com.wavesplatform.state.diffs +import scala.util.{Left, Right} + import cats._ +import cats.data.Validated +import cats.instances.lazyList._ +import cats.syntax.traverse._ import com.wavesplatform.account.{Address, AddressScheme} -import com.wavesplatform.features.OverdraftValidationProvider._ import com.wavesplatform.features.{BlockchainFeature, BlockchainFeatures} +import com.wavesplatform.features.OverdraftValidationProvider._ import com.wavesplatform.lang.ValidationError import com.wavesplatform.lang.directives.values._ +import com.wavesplatform.lang.script.{ContractScript, Script} import com.wavesplatform.lang.script.ContractScript.ContractScriptImpl 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.transaction.Asset.{IssuedAsset, Waves} import com.wavesplatform.transaction.TxValidationError._ -import com.wavesplatform.transaction._ import com.wavesplatform.transaction.assets._ import com.wavesplatform.transaction.assets.exchange._ import com.wavesplatform.transaction.lease._ -import com.wavesplatform.transaction.smart.InvokeScriptTransaction.Payment import com.wavesplatform.transaction.smart.{InvokeScriptTransaction, SetScriptTransaction} +import com.wavesplatform.transaction.smart.InvokeScriptTransaction.Payment import com.wavesplatform.transaction.transfer._ -import scala.util.{Left, Right} - object CommonValidation { - def disallowSendingGreaterThanBalance[T <: Transaction](blockchain: Blockchain, blockTime: Long, tx: T): Either[ValidationError, T] = if (blockTime >= blockchain.settings.functionalitySettings.allowTemporaryNegativeUntil) { def checkTransfer( @@ -106,6 +108,22 @@ object CommonValidation { } } else Right(tx) + def disallowNegativeBalances(blockchain: Blockchain, diff: Diff): Either[ValidationError, Diff] = { + val resultBalances = for { + (address, pf) <- diff.portfolios.to(LazyList) + (asset, delta) <- pf.assets ++ Map(Waves -> pf.balance) + newBalance = safeSum(blockchain.balance(address, asset), delta) + } yield (address, asset, newBalance) + + resultBalances.collect { + case tuple @ (_, _, balance) if balance < 0 => Validated.invalid(tuple) + case _ => Validated.valid(()) + }.map(_.toValidatedNel).sequence match { + case Validated.Valid(_) => Right(diff) + case Validated.Invalid(tuples) => Left(GenericError(s"Diff contains negative applied balances: ${tuples.toList.mkString("[", ", ", "]")}")) + } + } + def disallowDuplicateIds[T <: Transaction](blockchain: Blockchain, tx: T): Either[ValidationError, T] = tx match { case _: PaymentTransaction => Right(tx) case _ => diff --git a/node/src/test/scala/com/wavesplatform/consensus/FairPoSCalculatorTest.scala b/node/src/test/scala/com/wavesplatform/consensus/FairPoSCalculatorTest.scala index 507bc10dadc..00bc6770159 100644 --- a/node/src/test/scala/com/wavesplatform/consensus/FairPoSCalculatorTest.scala +++ b/node/src/test/scala/com/wavesplatform/consensus/FairPoSCalculatorTest.scala @@ -1,5 +1,8 @@ package com.wavesplatform.consensus +import scala.io.Source +import scala.util.Random + import cats.data.NonEmptyList import com.wavesplatform.account.{KeyPair, PrivateKey, PublicKey} import com.wavesplatform.common.state.ByteStr @@ -7,9 +10,6 @@ import com.wavesplatform.common.utils.{Base58, EitherExt2} import com.wavesplatform.crypto import com.wavesplatform.test.PropSpec -import scala.io.Source -import scala.util.Random - class FairPoSCalculatorTest extends PropSpec { import FairPoSCalculatorTest._ import PoSCalculator._ @@ -33,6 +33,11 @@ class FairPoSCalculatorTest extends PropSpec { baseTarget shouldBe 1 } + property("Base target should not be overflowed") { + val baseTarget = FairPoSCalculator(60, 0).calculateBaseTarget(60, 1, Long.MaxValue, 2, Some(1), 3e10.toLong) + baseTarget shouldBe Long.MaxValue + } + property("Correct consensus parameters distribution of blocks generated with FairPoS") { val miners = mkMiners @@ -133,8 +138,8 @@ class FairPoSCalculatorTest extends PropSpec { object FairPoSCalculatorTest { import play.api.libs.functional.syntax._ - import play.api.libs.json.Reads._ import play.api.libs.json._ + import play.api.libs.json.Reads._ case class Input( privateKey: PrivateKey, diff --git a/node/src/test/scala/com/wavesplatform/db/WithState.scala b/node/src/test/scala/com/wavesplatform/db/WithState.scala index f9cc1e684dd..828b8decf85 100644 --- a/node/src/test/scala/com/wavesplatform/db/WithState.scala +++ b/node/src/test/scala/com/wavesplatform/db/WithState.scala @@ -3,24 +3,24 @@ package com.wavesplatform.db import java.nio.file.Files import cats.Monoid +import com.wavesplatform.{NTPTime, TestHelpers} import com.wavesplatform.account.Address import com.wavesplatform.block.Block import com.wavesplatform.common.utils.EitherExt2 -import com.wavesplatform.database.{LevelDBFactory, LevelDBWriter, TestStorageFactory, loadActiveLeases} +import com.wavesplatform.database.{loadActiveLeases, LevelDBFactory, LevelDBWriter, TestStorageFactory} import com.wavesplatform.events.BlockchainUpdateTriggers import com.wavesplatform.features.{BlockchainFeature, BlockchainFeatures} import com.wavesplatform.history.Domain import com.wavesplatform.lagonaki.mocks.TestBlock import com.wavesplatform.lang.ValidationError import com.wavesplatform.mining.MiningConstraint -import com.wavesplatform.settings.{BlockchainSettings, FunctionalitySettings, TestSettings, WavesSettings, loadConfig, TestFunctionalitySettings => TFS} -import com.wavesplatform.state.diffs.{BlockDiffer, produce} +import com.wavesplatform.settings.{loadConfig, BlockchainSettings, FunctionalitySettings, TestSettings, WavesSettings, TestFunctionalitySettings => TFS} +import com.wavesplatform.state.{Blockchain, BlockchainUpdaterImpl, Diff} +import com.wavesplatform.state.diffs.{produce, BlockDiffer} import com.wavesplatform.state.reader.CompositeBlockchain import com.wavesplatform.state.utils.TestLevelDB -import com.wavesplatform.state.{Blockchain, BlockchainUpdaterImpl, Diff} -import com.wavesplatform.transaction.smart.script.trace.TracedResult import com.wavesplatform.transaction.{Asset, Transaction} -import com.wavesplatform.{NTPTime, TestHelpers} +import com.wavesplatform.transaction.smart.script.trace.TracedResult import monix.reactive.Observer import monix.reactive.subjects.{PublishSubject, Subject} import org.iq80.leveldb.{DB, Options} @@ -193,6 +193,8 @@ trait WithDomain extends WithState { _: Suite => BlockchainFeatures.BlockV5 ) + val RideV4WithRewards = RideV4.addFeatures(BlockchainFeatures.BlockReward) + val RideV5 = RideV4.addFeatures(BlockchainFeatures.SynchronousCalls) } diff --git a/node/src/test/scala/com/wavesplatform/history/Domain.scala b/node/src/test/scala/com/wavesplatform/history/Domain.scala index 898b25002aa..b655d79263b 100644 --- a/node/src/test/scala/com/wavesplatform/history/Domain.scala +++ b/node/src/test/scala/com/wavesplatform/history/Domain.scala @@ -224,6 +224,17 @@ case class Domain(db: DB, blockchainUpdater: BlockchainUpdaterImpl, levelDBWrite CommonBlocksApi(blockchainUpdater, loadBlockMetaAt(db, blockchainUpdater), loadBlockInfoAt(db, blockchainUpdater)) } + + //noinspection ScalaStyle + object helpers { + def creditWaves(address: Address, amount: Long = 10_0000_0000): Unit = { + appendBlock(TxHelpers.genesis(address, amount)) + } + + def creditWavesToDefaultSigner(amount: Long = 10_0000_0000): Unit = { + creditWaves(TxHelpers.defaultAddress, amount) + } + } } object Domain { diff --git a/node/src/test/scala/com/wavesplatform/mining/MinerBalanceOverflowTest.scala b/node/src/test/scala/com/wavesplatform/mining/MinerBalanceOverflowTest.scala new file mode 100644 index 00000000000..4b13d7f8101 --- /dev/null +++ b/node/src/test/scala/com/wavesplatform/mining/MinerBalanceOverflowTest.scala @@ -0,0 +1,15 @@ +package com.wavesplatform.mining + +import com.wavesplatform.db.WithDomain +import com.wavesplatform.test.FlatSpec +import com.wavesplatform.transaction.TxHelpers +import org.scalatest.matchers.should.Matchers + +class MinerBalanceOverflowTest extends FlatSpec with Matchers with WithDomain { + "Miner balance" should "not overflow" in withDomain(DomainPresets.RideV4WithRewards) { d => + d.helpers.creditWavesToDefaultSigner(Long.MaxValue - 6_0000_0000L) + for (_ <- 1 to 10) intercept[RuntimeException](d.appendBlock()).toString should include("Diff contains negative applied balances") + val minerBalance = d.blockchain.balance(TxHelpers.defaultAddress) + minerBalance shouldBe >(0L) + } +}