From e311af4ebe424e25b1aea788205c95bf46c5767a Mon Sep 17 00:00:00 2001 From: Vyatcheslav Suharnikov Date: Fri, 20 Oct 2023 13:35:46 +0400 Subject: [PATCH] NODE-2619 Feature activation fixes (#3900) --- .../com/wavesplatform/database/Caches.scala | 3 +- .../database/LevelDBWriter.scala | 13 +- .../transaction/BlockchainUpdaterTest.scala | 277 ++++++++++++++++-- 3 files changed, 260 insertions(+), 33 deletions(-) diff --git a/node/src/main/scala/com/wavesplatform/database/Caches.scala b/node/src/main/scala/com/wavesplatform/database/Caches.scala index c528394d888..8cc2f9188c8 100644 --- a/node/src/main/scala/com/wavesplatform/database/Caches.scala +++ b/node/src/main/scala/com/wavesplatform/database/Caches.scala @@ -1,6 +1,5 @@ package com.wavesplatform.database -import java.util import cats.data.Ior import cats.syntax.monoid.* import cats.syntax.option.* @@ -20,6 +19,7 @@ import com.wavesplatform.transaction.{Asset, Transaction} import com.wavesplatform.utils.ObservedLoadingCache import monix.reactive.Observer +import java.util import scala.collection.immutable.VectorMap import scala.concurrent.duration.* import scala.jdk.CollectionConverters.* @@ -152,6 +152,7 @@ abstract class Caches(spendableBalanceChanged: Observer[(Address, Asset)]) exten protected def loadApprovedFeatures(): Map[Short, Int] override def approvedFeatures: Map[Short, Int] = approvedFeaturesCache + // Also contains features those will be activated in the future (activationHeight > currentHeight), because they were approved now or before. @volatile protected var activatedFeaturesCache: Map[Short, Int] = loadActivatedFeatures() protected def loadActivatedFeatures(): Map[Short, Int] diff --git a/node/src/main/scala/com/wavesplatform/database/LevelDBWriter.scala b/node/src/main/scala/com/wavesplatform/database/LevelDBWriter.scala index b73861aad01..8fcc02e4645 100644 --- a/node/src/main/scala/com/wavesplatform/database/LevelDBWriter.scala +++ b/node/src/main/scala/com/wavesplatform/database/LevelDBWriter.scala @@ -530,10 +530,10 @@ abstract class LevelDBWriter private[database] ( } if (newlyApprovedFeatures.nonEmpty) { - approvedFeaturesCache = newlyApprovedFeatures ++ rw.get(Keys.approvedFeatures) + approvedFeaturesCache = newlyApprovedFeatures ++ approvedFeaturesCache rw.put(Keys.approvedFeatures, approvedFeaturesCache) - val featuresToSave = (newlyApprovedFeatures.view.mapValues(_ + activationWindowSize) ++ rw.get(Keys.activatedFeatures)).toMap + val featuresToSave = (newlyApprovedFeatures.view.mapValues(_ + activationWindowSize) ++ activatedFeaturesCache).toMap activatedFeaturesCache = featuresToSave ++ settings.functionalitySettings.preActivatedFeatures rw.put(Keys.activatedFeatures, featuresToSave) @@ -750,6 +750,15 @@ abstract class LevelDBWriter private[database] ( disabledAliases = DisableHijackedAliases.revert(rw) } + val disapprovedFeatures = approvedFeaturesCache.collect { case (id, approvalHeight) if approvalHeight > targetHeight => id } + if (disapprovedFeatures.nonEmpty) { + approvedFeaturesCache --= disapprovedFeatures + rw.put(Keys.approvedFeatures, approvedFeaturesCache) + + activatedFeaturesCache --= disapprovedFeatures // We won't activate them in the future + rw.put(Keys.activatedFeatures, activatedFeaturesCache) + } + val hitSource = rw.get(Keys.hitSource(currentHeight)).get val block = createBlock(discardedMeta.header, discardedMeta.signature, loadTransactions(currentHeight, rw).map(_._2)).explicitGet() diff --git a/node/src/test/scala/com/wavesplatform/transaction/BlockchainUpdaterTest.scala b/node/src/test/scala/com/wavesplatform/transaction/BlockchainUpdaterTest.scala index c02f3eb4ea7..89df20fc3dd 100644 --- a/node/src/test/scala/com/wavesplatform/transaction/BlockchainUpdaterTest.scala +++ b/node/src/test/scala/com/wavesplatform/transaction/BlockchainUpdaterTest.scala @@ -16,12 +16,13 @@ import scala.util.Try class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { - private val ApprovalPeriod = 100 + private val ApprovalPeriod = 100 + private val BlocksForActivation = (ApprovalPeriod * 0.9).toInt private val WavesSettings = history.DefaultWavesSettings.copy( blockchainSettings = history.DefaultWavesSettings.blockchainSettings.copy( functionalitySettings = history.DefaultWavesSettings.blockchainSettings.functionalitySettings - .copy(featureCheckBlocksPeriod = ApprovalPeriod, blocksForFeatureActivation = (ApprovalPeriod * 0.9).toInt, preActivatedFeatures = Map.empty) + .copy(featureCheckBlocksPeriod = ApprovalPeriod, blocksForFeatureActivation = BlocksForActivation, preActivatedFeatures = Map.empty) ), featuresSettings = history.DefaultWavesSettings.featuresSettings.copy(autoShutdownOnUnsupportedFeature = true) ) @@ -65,7 +66,7 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(3, 2 * ApprovalPeriod) shouldBe BlockchainFeatureStatus.Undefined (1 to ApprovalPeriod).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) + b.processBlock(getNextTestBlock(b)) } b.height shouldBe 3 * ApprovalPeriod @@ -128,7 +129,208 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(2, ApprovalPeriod - 1) shouldBe BlockchainFeatureStatus.Undefined } - "feature activation height is not overriden with further periods" in withDomain(WavesSettings) { domain => + "multiple features activation: one after another" - { + def appendBlocks(b: BlockchainUpdaterImpl, blocks: Int, votes: Short*): Unit = (1 to blocks).foreach { _ => + b.processBlock(getNextTestBlockWithVotes(b, votes)) should beRight + } + + def check( + b: BlockchainUpdaterImpl, + height: Int, + approvedFeatures: Map[Int, Int] = Map.empty, + activatedFeatures: Map[Int, Int] = Map.empty + ): Unit = { + b.height shouldBe height + withClue("approved:") { + b.approvedFeatures shouldBe approvedFeatures + } + withClue("activated:") { + b.activatedFeatures shouldBe activatedFeatures + } + } + + "without votes for already activated feature" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + b.processBlock(genesisBlock) + + check(b, 1) + + markup("Approving the first feature") + appendBlocks(b, blocks = ApprovalPeriod - 1, votes = 1) + check( + b, + ApprovalPeriod, + approvedFeatures = Map(1 -> ApprovalPeriod), + activatedFeatures = Map(1 -> ApprovalPeriod * 2) + ) + + markup("Approving the second feature without voting for first feature") + appendBlocks(b, blocks = ApprovalPeriod, votes = 2) + check( + b, + ApprovalPeriod * 2, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + + markup("Activating the second feature") + appendBlocks(b, blocks = ApprovalPeriod) + check( + b, + ApprovalPeriod * 3, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + } + + "with votes for already activated feature" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + b.processBlock(genesisBlock) + + markup("Approving the first feature") + appendBlocks(b, blocks = ApprovalPeriod - 1, votes = 1) + + markup("Approving the second feature, still voting for first feature") + appendBlocks(b, blocks = ApprovalPeriod, votes = 1, 2) + check( + b, + ApprovalPeriod * 2, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + + markup("Activating the second feature") + appendBlocks(b, blocks = ApprovalPeriod) + check( + b, + ApprovalPeriod * 3, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + } + + "with multiple voting periods rollback" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + b.processBlock(genesisBlock) + + markup("Approving the first feature") + appendBlocks(b, blocks = ApprovalPeriod - 1, votes = 1) + + markup("Approving the second feature") + appendBlocks(b, blocks = ApprovalPeriod, votes = 2) + + markup("Approving the third feature") + appendBlocks(b, blocks = ApprovalPeriod, votes = 3) + + markup("Approving the fourth feature") + appendBlocks(b, blocks = ApprovalPeriod, votes = 4) + check( + b, + ApprovalPeriod * 4, + approvedFeatures = Map( + 1 -> ApprovalPeriod, + 2 -> ApprovalPeriod * 2, + 3 -> ApprovalPeriod * 3, + 4 -> ApprovalPeriod * 4 + ), + activatedFeatures = Map( + 1 -> ApprovalPeriod * 2, + 2 -> ApprovalPeriod * 3, + 3 -> ApprovalPeriod * 4, + 4 -> ApprovalPeriod * 5 // Actually, it is not activated, because we haven't reach this height + ) + ) + + markup("Rollback") + val rollbackHeight = ApprovalPeriod * 2 + 1 + b.removeAfter(b.blockHeader(rollbackHeight).get.id()).explicitGet() + check( + b, + rollbackHeight, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + + markup("Appending blocks without votes to reach the previous height") + appendBlocks(b, blocks = ApprovalPeriod * 2 - 1) + check( + b, + ApprovalPeriod * 4, + approvedFeatures = Map(1 -> ApprovalPeriod, 2 -> ApprovalPeriod * 2), + activatedFeatures = Map(1 -> ApprovalPeriod * 2, 2 -> ApprovalPeriod * 3) + ) + } + } + + "features activation after rollback and appending blocks" - { + def appendAndRollback(b: BlockchainUpdaterImpl, rollbackToHeight: Int): Unit = { + b.processBlock(genesisBlock) + + markup("Approving the feature") + b.featureStatus(1, 1) shouldBe BlockchainFeatureStatus.Undefined + (1 until ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlockWithVotes(b, Seq(1))) should beRight + } + + b.height shouldBe ApprovalPeriod + b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Approved + + markup("Appending blocks without votes after approval") + (1 to 2).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + markup("Rollback before approval height") + b.removeAfter(b.blockHeader(rollbackToHeight).get.id()).explicitGet() + b.featureStatus(1, rollbackToHeight) shouldBe BlockchainFeatureStatus.Undefined + } + + "when not enough votes after rollback - should not approve and activate the feature" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + appendAndRollback(b, BlocksForActivation) // 1st blocks is genesis, BlocksForActivation - 1 votes + + markup("Appending blocks to reach ApprovalPeriod height") + (b.height until ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + b.height shouldBe ApprovalPeriod + b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Undefined + + markup("Appending blocks to reach ApprovalPeriod * 2") + (1 to ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + val newHeight = ApprovalPeriod * 2 + b.height shouldBe newHeight + b.featureStatus(1, newHeight) shouldBe BlockchainFeatureStatus.Undefined + } + + "when enough votes after rollback - should approve and activate the feature" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + appendAndRollback(b, BlocksForActivation + 1) // 1st blocks is genesis, BlocksForActivation - 1 + 1 votes + + markup("Appending blocks to reach ApprovalPeriod height") + (b.height until ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + b.height shouldBe ApprovalPeriod + b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Approved + + markup("Appending blocks to reach ApprovalPeriod * 2") + (1 to ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + val activationHeight = ApprovalPeriod * 2 + b.height shouldBe activationHeight + b.featureStatus(1, activationHeight) shouldBe BlockchainFeatureStatus.Activated + } + } + + "feature activation height is not overridden with further periods" in withDomain(WavesSettings) { domain => val b = domain.blockchainUpdater b.processBlock(genesisBlock) @@ -150,27 +352,42 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureActivationHeight(1) shouldBe Some(ApprovalPeriod * 2) } - "feature activated only by 90% of blocks" in withDomain(WavesSettings) { domain => - val b = domain.blockchainUpdater + "feature activated only by 90% of blocks" - { + "negative with insufficient votes" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + b.processBlock(genesisBlock) - b.processBlock(genesisBlock) + (1 until BlocksForActivation).foreach { _ => + b.processBlock(getNextTestBlockWithVotes(b, Seq(1))) should beRight + } - b.featureStatus(1, 1) shouldBe BlockchainFeatureStatus.Undefined + (BlocksForActivation to ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } - (1 until ApprovalPeriod).foreach { i => - b.processBlock(getNextTestBlockWithVotes(b, if (i % 2 == 0) Seq(1) else Seq())) should beRight - } - b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Undefined + b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Undefined - (1 to ApprovalPeriod).foreach { i => - b.processBlock(getNextTestBlockWithVotes(b, if (i % 10 == 0) Seq() else Seq(1))) should beRight + (1 to ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + + b.featureStatus(1, ApprovalPeriod * 2) shouldBe BlockchainFeatureStatus.Undefined } - b.featureStatus(1, ApprovalPeriod * 2) shouldBe BlockchainFeatureStatus.Approved - (1 to ApprovalPeriod).foreach { i => - b.processBlock(getNextTestBlock(b)) should beRight + "positive" in withDomain(WavesSettings) { domain => + val b = domain.blockchainUpdater + b.processBlock(genesisBlock) + + (1 to ApprovalPeriod).foreach { i => + b.processBlock(getNextTestBlockWithVotes(b, if (i % 10 == 0) Seq() else Seq(1))) should beRight + } + b.featureStatus(1, ApprovalPeriod) shouldBe BlockchainFeatureStatus.Approved + + (1 to ApprovalPeriod).foreach { _ => + b.processBlock(getNextTestBlock(b)) should beRight + } + b.featureStatus(1, ApprovalPeriod * 2) shouldBe BlockchainFeatureStatus.Activated } - b.featureStatus(1, ApprovalPeriod * 3) shouldBe BlockchainFeatureStatus.Activated } "features votes resets when voting window changes" in withDomain(WavesSettings) { domain => @@ -256,15 +473,15 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Approved) (0 until ApprovalPeriod - 1).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Approved) - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Activated) (0 until ApprovalPeriod * 2).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } (0 until ApprovalPeriod - 1).foreach { _ => @@ -275,10 +492,10 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Approved) (0 until ApprovalPeriod - 1).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Approved) - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Activated) } @@ -305,10 +522,10 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { // 300 blocks passed, the activation period should be doubled now (0 until ApprovalPeriod - 1).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Approved) - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Activated) } @@ -326,15 +543,15 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Approved) (0 until ApprovalPeriod - 1).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Approved) - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight b.featureStatus(1, b.height) should be(BlockchainFeatureStatus.Activated) (0 until ApprovalPeriod * 2).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } (0 until ApprovalPeriod * 2 - 1).foreach { _ => @@ -345,10 +562,10 @@ class BlockchainUpdaterTest extends FreeSpec with HistoryTest with WithDomain { b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Approved) (0 until ApprovalPeriod * 2 - 1).foreach { _ => - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight } b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Approved) - b.processBlock(getNextTestBlockWithVotes(b, Seq())) should beRight + b.processBlock(getNextTestBlock(b)) should beRight b.featureStatus(2, b.height) should be(BlockchainFeatureStatus.Activated) }