From 282f6e6e2a6cb8e93c4fc32014cca6adab38a623 Mon Sep 17 00:00:00 2001 From: catbref Date: Tue, 18 Feb 2020 13:23:42 +0000 Subject: [PATCH] Changed error response from RewardShareTransaction.isValid() for existing self-shares. Before: 0-fee self-share REWARD_SHARE when there is an existing self-share would result in INSUFFICIENT_FEE from isFeeValid() After: isFeeValid() returns OK for above, but isValid() returns SELF_SHARE_EXISTS. In addition, a transaction that tries to modify existing self-share, even with fee, also returns SELF_SHARE_EXISTS. Improved tests to double check. --- .../org/qortal/transaction/RewardShareTransaction.java | 7 ++++--- src/main/java/org/qortal/transaction/Transaction.java | 1 + .../java/org/qortal/test/minting/RewardShareTests.java | 9 +++++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/qortal/transaction/RewardShareTransaction.java b/src/main/java/org/qortal/transaction/RewardShareTransaction.java index e30b15f2..cefb1e4a 100644 --- a/src/main/java/org/qortal/transaction/RewardShareTransaction.java +++ b/src/main/java/org/qortal/transaction/RewardShareTransaction.java @@ -102,9 +102,10 @@ public class RewardShareTransaction extends Transaction { return ValidationResult.INVALID_PUBLIC_KEY; final boolean isRecipientAlsoMinter = getCreator().getAddress().equals(this.rewardShareTransactionData.getRecipient()); + final boolean isCancellingSharePercent = this.rewardShareTransactionData.getSharePercent().compareTo(BigDecimal.ZERO) < 0; - // Fee can be zero if setting up new self-share - if (isRecipientAlsoMinter && existingRewardShareData == null && this.transactionData.getFee().compareTo(BigDecimal.ZERO) >= 0) + // Fee can be zero if self-share, and not cancelling + if (isRecipientAlsoMinter && !isCancellingSharePercent && this.transactionData.getFee().compareTo(BigDecimal.ZERO) >= 0) return ValidationResult.OK; return super.isFeeValid(); @@ -161,7 +162,7 @@ public class RewardShareTransaction extends Transaction { // Modifying an existing self-share is pointless and forbidden (due to 0 fee). Deleting self-share is OK though. if (isRecipientAlsoMinter && !isCancellingSharePercent) - return ValidationResult.INVALID_REWARD_SHARE_PERCENT; + return ValidationResult.SELF_SHARE_EXISTS; } // Fee checking needed if not setting up new self-share diff --git a/src/main/java/org/qortal/transaction/Transaction.java b/src/main/java/org/qortal/transaction/Transaction.java index f3cba5ea..adf70310 100644 --- a/src/main/java/org/qortal/transaction/Transaction.java +++ b/src/main/java/org/qortal/transaction/Transaction.java @@ -240,6 +240,7 @@ public abstract class Transaction { CLOCK_NOT_SYNCED(88), ASSET_NOT_SPENDABLE(89), ACCOUNT_CANNOT_REWARD_SHARE(90), + SELF_SHARE_EXISTS(91), NOT_YET_RELEASED(1000); public final int value; diff --git a/src/test/java/org/qortal/test/minting/RewardShareTests.java b/src/test/java/org/qortal/test/minting/RewardShareTests.java index cb0afd71..04e208b0 100644 --- a/src/test/java/org/qortal/test/minting/RewardShareTests.java +++ b/src/test/java/org/qortal/test/minting/RewardShareTests.java @@ -168,9 +168,14 @@ public class RewardShareTests extends Common { newTransactionData = AccountUtils.createRewardShare(repository, testAccountName, testAccountName, CANCEL_SHARE_PERCENT); newTransaction = Transaction.fromData(repository, newTransactionData); - // Confirm terminating reward-share is valid + // Confirm terminating reward-share with fee is valid validationResult = newTransaction.isValidUnconfirmed(); - assertEquals("Subsequent zero-fee self-share should be invalid", ValidationResult.OK, validationResult); + assertEquals("Subsequent self-share cancel should be valid", ValidationResult.OK, validationResult); + + // Confirm terminating reward-share with zero fee is invalid + newTransactionData.setFee(BigDecimal.ZERO); + validationResult = newTransaction.isValidUnconfirmed(); + assertNotSame("Subsequent zero-fee self-share cancel should be invalid", ValidationResult.OK, validationResult); } }