From b800fb5846c8cddb949ad03588df1b6da438b04d Mon Sep 17 00:00:00 2001 From: CalDescent Date: Thu, 9 Sep 2021 12:54:01 +0100 Subject: [PATCH] Treat a REGISTER_NAME transaction as an UPDATE_NAME if the creator matches. Whilst not ideal, this is necessary to prevent the chain from getting stuck on future blocks due to duplicate name registrations. See Block535658.java for full details on this problem - this is simply a "catch-all" implementation of that class in order to futureproof this fix. There is still a database inconsistency to be solved, as some nodes are failing to add a registered name to their Names table the first time around, but this will take some time. Once fixed, this commit could potentially be reverted. Also added unit tests for both scenarios (same and different creator). TLDR: this allows all past and future invalid blocks caused by NAME_ALREADY_REGISTERED (by the same creator) to now be valid. --- .../transaction/RegisterNameTransaction.java | 22 +++++++++++++- .../org/qortal/test/naming/MiscTests.java | 30 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/qortal/transaction/RegisterNameTransaction.java b/src/main/java/org/qortal/transaction/RegisterNameTransaction.java index 66c1fc8b..ad20ef1c 100644 --- a/src/main/java/org/qortal/transaction/RegisterNameTransaction.java +++ b/src/main/java/org/qortal/transaction/RegisterNameTransaction.java @@ -2,11 +2,13 @@ package org.qortal.transaction; import java.util.Collections; import java.util.List; +import java.util.Objects; import org.qortal.account.Account; import org.qortal.asset.Asset; import org.qortal.block.BlockChain; import org.qortal.crypto.Crypto; +import org.qortal.data.naming.NameData; import org.qortal.data.transaction.RegisterNameTransactionData; import org.qortal.data.transaction.TransactionData; import org.qortal.naming.Name; @@ -77,14 +79,32 @@ public class RegisterNameTransaction extends Transaction { @Override public ValidationResult isProcessable() throws DataException { // Check the name isn't already taken - if (this.repository.getNameRepository().reducedNameExists(this.registerNameTransactionData.getReducedName())) + if (this.repository.getNameRepository().reducedNameExists(this.registerNameTransactionData.getReducedName())) { + // Name exists, but we'll allow the transaction if it has the same creator + // This is necessary to workaround an issue due to inconsistent data in the Names table on some nodes. + // Without this, the chain can get stuck for a subset of nodes when the name is registered + // for the second time. It's simplest to just treat REGISTER_NAME as UPDATE_NAME if the creator + // matches that of the original registration. + + NameData nameData = this.repository.getNameRepository().fromReducedName(this.registerNameTransactionData.getReducedName()); + if (Objects.equals(this.getCreator().getAddress(), nameData.getOwner())) { + // Transaction creator already owns the name, so it's safe to update it + // Treat this as valid, which also requires skipping the "one name per account" check below. + // Given that the name matches one already registered, we know that it won't exceed the limit. + return ValidationResult.OK; + } + + // Name is already registered to someone else return ValidationResult.NAME_ALREADY_REGISTERED; + } // If accounts are only allowed one registered name then check for this if (BlockChain.getInstance().oneNamePerAccount() && !this.repository.getNameRepository().getNamesByOwner(getRegistrant().getAddress()).isEmpty()) return ValidationResult.MULTIPLE_NAMES_FORBIDDEN; + // FUTURE: when adding more validation, make sure to check the `return ValidationResult.OK` above + return ValidationResult.OK; } diff --git a/src/test/java/org/qortal/test/naming/MiscTests.java b/src/test/java/org/qortal/test/naming/MiscTests.java index c46cbfab..23e0e720 100644 --- a/src/test/java/org/qortal/test/naming/MiscTests.java +++ b/src/test/java/org/qortal/test/naming/MiscTests.java @@ -44,9 +44,9 @@ public class MiscTests extends Common { } } - // test trying to register same name twice + // test trying to register same name twice (with same creator) @Test - public void testDuplicateRegisterName() throws DataException { + public void testDuplicateRegisterNameWithSameCreator() throws DataException { try (final Repository repository = RepositoryManager.getRepository()) { // Register-name PrivateKeyAccount alice = Common.getTestAccount(repository, "alice"); @@ -63,7 +63,31 @@ public class MiscTests extends Common { transaction.sign(alice); ValidationResult result = transaction.importAsUnconfirmed(); - assertTrue("Transaction should be invalid", ValidationResult.OK != result); + assertTrue("Transaction should be valid because it has the same creator", ValidationResult.OK == result); + } + } + + // test trying to register same name twice (with different creator) + @Test + public void testDuplicateRegisterNameWithDifferentCreator() throws DataException { + try (final Repository repository = RepositoryManager.getRepository()) { + // Register-name + PrivateKeyAccount alice = Common.getTestAccount(repository, "alice"); + String name = "test-name"; + String data = "{}"; + + RegisterNameTransactionData transactionData = new RegisterNameTransactionData(TestTransaction.generateBase(alice), name, data); + TransactionUtils.signAndMint(repository, transactionData, alice); + + // duplicate (this time registered by Bob) + PrivateKeyAccount bob = Common.getTestAccount(repository, "bob"); + String duplicateName = "TEST-nÁme"; + transactionData = new RegisterNameTransactionData(TestTransaction.generateBase(bob), duplicateName, data); + Transaction transaction = Transaction.fromData(repository, transactionData); + transaction.sign(alice); + + ValidationResult result = transaction.importAsUnconfirmed(); + assertTrue("Transaction should be invalid because it has a different creator", ValidationResult.OK != result); } }