mirror of
https://github.com/Qortal/qortal.git
synced 2025-07-22 20:26:50 +00:00
Self-reward-share tests and fixes.
Added Transaction.isFeeValid() to allow transaction subclasses to override and allow zero fees, etc. Added tests to cover self-reward-shares, including zero fee scenario. Set 'dilbert' test account to level 8 in test genesis block. Removed leftover mention of "previous_level" from HSQLDBAccountLevelTransactions, and AccountLevelTransactionData. (Previous level makes no sense as ACCOUNT_LEVEL transactions are genesis-block only). Fixed some incorrect uses of PrivateKeyAccount.getSharedSecret() to PrivateKeyAccount.getRewardSharePrivateKey() in tests.
This commit is contained in:
@@ -21,7 +21,6 @@ public class AccountLevelTransactionData extends TransactionData {
|
||||
|
||||
private String target;
|
||||
private int level;
|
||||
private Integer previousLevel;
|
||||
|
||||
// Constructors
|
||||
|
||||
@@ -40,20 +39,13 @@ public class AccountLevelTransactionData extends TransactionData {
|
||||
this.creatorPublicKey = GenesisAccount.PUBLIC_KEY;
|
||||
}
|
||||
|
||||
/** From repository */
|
||||
/** From repository, network/API */
|
||||
public AccountLevelTransactionData(BaseTransactionData baseTransactionData,
|
||||
String target, int level, Integer previousLevel) {
|
||||
String target, int level) {
|
||||
super(TransactionType.ACCOUNT_LEVEL, baseTransactionData);
|
||||
|
||||
this.target = target;
|
||||
this.level = level;
|
||||
this.previousLevel = previousLevel;
|
||||
}
|
||||
|
||||
/** From network/API */
|
||||
public AccountLevelTransactionData(BaseTransactionData baseTransactionData,
|
||||
String target, int level) {
|
||||
this(baseTransactionData, target, level, null);
|
||||
}
|
||||
|
||||
// Getters / setters
|
||||
@@ -66,14 +58,6 @@ public class AccountLevelTransactionData extends TransactionData {
|
||||
return this.level;
|
||||
}
|
||||
|
||||
public Integer getPreviousLevel() {
|
||||
return this.previousLevel;
|
||||
}
|
||||
|
||||
public void setPreviousLevel(Integer previousLevel) {
|
||||
this.previousLevel = previousLevel;
|
||||
}
|
||||
|
||||
// Re-expose to JAXB
|
||||
|
||||
@XmlElement(name = "creatorPublicKey")
|
||||
|
@@ -17,7 +17,7 @@ public class HSQLDBAccountLevelTransactionRepository extends HSQLDBTransactionRe
|
||||
}
|
||||
|
||||
TransactionData fromBase(BaseTransactionData baseTransactionData) throws DataException {
|
||||
String sql = "SELECT target, level, previous_level FROM AccountLevelTransactions WHERE signature = ?";
|
||||
String sql = "SELECT target, level FROM AccountLevelTransactions WHERE signature = ?";
|
||||
|
||||
try (ResultSet resultSet = this.repository.checkedExecute(sql, baseTransactionData.getSignature())) {
|
||||
if (resultSet == null)
|
||||
@@ -26,11 +26,7 @@ public class HSQLDBAccountLevelTransactionRepository extends HSQLDBTransactionRe
|
||||
String target = resultSet.getString(1);
|
||||
int level = resultSet.getInt(2);
|
||||
|
||||
Integer previousLevel = resultSet.getInt(3);
|
||||
if (previousLevel == 0 && resultSet.wasNull())
|
||||
previousLevel = null;
|
||||
|
||||
return new AccountLevelTransactionData(baseTransactionData, target, level, previousLevel);
|
||||
return new AccountLevelTransactionData(baseTransactionData, target, level);
|
||||
} catch (SQLException e) {
|
||||
throw new DataException("Unable to fetch account level transaction from repository", e);
|
||||
}
|
||||
|
@@ -61,6 +61,22 @@ public class RewardShareTransaction extends Transaction {
|
||||
return amount;
|
||||
}
|
||||
|
||||
private RewardShareData getExistingRewardShare() throws DataException {
|
||||
// Look up any existing reward-share (using transaction's reward-share public key)
|
||||
RewardShareData existingRewardShareData = this.repository.getAccountRepository().getRewardShare(this.rewardShareTransactionData.getRewardSharePublicKey());
|
||||
if (existingRewardShareData == null)
|
||||
// No luck, try looking up existing reward-share using minting & recipient account info
|
||||
existingRewardShareData = this.repository.getAccountRepository().getRewardShare(this.rewardShareTransactionData.getMinterPublicKey(), this.rewardShareTransactionData.getRecipient());
|
||||
|
||||
return existingRewardShareData;
|
||||
}
|
||||
|
||||
private boolean doesRewardShareMatch(RewardShareData rewardShareData) {
|
||||
return rewardShareData.getRecipient().equals(this.rewardShareTransactionData.getRecipient())
|
||||
&& Arrays.equals(rewardShareData.getMinterPublicKey(), this.rewardShareTransactionData.getMinterPublicKey())
|
||||
&& Arrays.equals(rewardShareData.getRewardSharePublicKey(), this.rewardShareTransactionData.getRewardSharePublicKey());
|
||||
}
|
||||
|
||||
// Navigation
|
||||
|
||||
public PublicKeyAccount getMintingAccount() {
|
||||
@@ -75,6 +91,25 @@ public class RewardShareTransaction extends Transaction {
|
||||
|
||||
private static final BigDecimal MAX_SHARE = BigDecimal.valueOf(100).setScale(2);
|
||||
|
||||
@Override
|
||||
public ValidationResult isFeeValid() throws DataException {
|
||||
// Look up any existing reward-share (using transaction's reward-share public key)
|
||||
RewardShareData existingRewardShareData = this.getExistingRewardShare();
|
||||
// If we have an existing reward-share then minter/recipient/reward-share-public-key should all match.
|
||||
// This is to prevent malicious actors using multiple (fake) reward-share public keys for the same minter/recipient combo,
|
||||
// or reusing the same reward-share public key for a different minter/recipient pair.
|
||||
if (existingRewardShareData != null && !this.doesRewardShareMatch(existingRewardShareData))
|
||||
return ValidationResult.INVALID_PUBLIC_KEY;
|
||||
|
||||
final boolean isRecipientAlsoMinter = getCreator().getAddress().equals(this.rewardShareTransactionData.getRecipient());
|
||||
|
||||
// Fee can be zero if setting up new self-share
|
||||
if (isRecipientAlsoMinter && existingRewardShareData == null && this.transactionData.getFee().compareTo(BigDecimal.ZERO) >= 0)
|
||||
return ValidationResult.OK;
|
||||
|
||||
return super.isFeeValid();
|
||||
}
|
||||
|
||||
@Override
|
||||
public ValidationResult isValid() throws DataException {
|
||||
// Check reward share given to recipient
|
||||
@@ -102,34 +137,25 @@ public class RewardShareTransaction extends Transaction {
|
||||
return ValidationResult.ACCOUNT_CANNOT_REWARD_SHARE;
|
||||
|
||||
// Look up any existing reward-share (using transaction's reward-share public key)
|
||||
RewardShareData rewardShareData = this.repository.getAccountRepository().getRewardShare(this.rewardShareTransactionData.getRewardSharePublicKey());
|
||||
if (rewardShareData != null) {
|
||||
// If reward-share public key already exists in repository, then it must be for the same minter-recipient combo
|
||||
if (!rewardShareData.getRecipient().equals(recipient.getAddress()) || !Arrays.equals(rewardShareData.getMinterPublicKey(), creator.getPublicKey()))
|
||||
return ValidationResult.INVALID_PUBLIC_KEY;
|
||||
|
||||
} else {
|
||||
// No luck, try looking up existing reward-share using minting & recipient account info
|
||||
rewardShareData = this.repository.getAccountRepository().getRewardShare(creator.getPublicKey(), recipient.getAddress());
|
||||
|
||||
if (rewardShareData != null)
|
||||
// If reward-share between minter & recipient already exists in repository, then it must be have the same public key
|
||||
if (!Arrays.equals(rewardShareData.getRewardSharePublicKey(), this.rewardShareTransactionData.getRewardSharePublicKey()))
|
||||
return ValidationResult.INVALID_PUBLIC_KEY;
|
||||
}
|
||||
RewardShareData existingRewardShareData = this.getExistingRewardShare();
|
||||
// If we have an existing reward-share then minter/recipient/reward-share-public-key should all match.
|
||||
// This is to prevent malicious actors using multiple (fake) reward-share public keys for the same minter/recipient combo,
|
||||
// or reusing the same reward-share public key for a different minter/recipient pair.
|
||||
if (existingRewardShareData != null && !this.doesRewardShareMatch(existingRewardShareData))
|
||||
return ValidationResult.INVALID_PUBLIC_KEY;
|
||||
|
||||
final boolean isSharePercentZero = this.rewardShareTransactionData.getSharePercent().compareTo(BigDecimal.ZERO) == 0;
|
||||
|
||||
if (rewardShareData == null) {
|
||||
if (existingRewardShareData == null) {
|
||||
// This is a new reward-share
|
||||
|
||||
// No point starting a new reward-share with 0% share (i.e. delete relationship)
|
||||
// No point starting a new reward-share with 0% share (i.e. delete reward-share)
|
||||
if (isSharePercentZero)
|
||||
return ValidationResult.INVALID_REWARD_SHARE_PERCENT;
|
||||
|
||||
// Check the minting account hasn't reach maximum number of reward-shares
|
||||
int relationshipCount = this.repository.getAccountRepository().countRewardShares(creator.getPublicKey());
|
||||
if (relationshipCount >= BlockChain.getInstance().getMaxRewardSharesPerMintingAccount())
|
||||
int rewardShareCount = this.repository.getAccountRepository().countRewardShares(creator.getPublicKey());
|
||||
if (rewardShareCount >= BlockChain.getInstance().getMaxRewardSharesPerMintingAccount())
|
||||
return ValidationResult.MAXIMUM_REWARD_SHARES;
|
||||
} else {
|
||||
// This transaction intends to modify/terminate an existing reward-share
|
||||
@@ -140,15 +166,10 @@ public class RewardShareTransaction extends Transaction {
|
||||
}
|
||||
|
||||
// Fee checking needed if not setting up new self-share
|
||||
if (!(isRecipientAlsoMinter && rewardShareData == null)) {
|
||||
// Check fee is positive
|
||||
if (rewardShareTransactionData.getFee().compareTo(BigDecimal.ZERO) <= 0)
|
||||
return ValidationResult.NEGATIVE_FEE;
|
||||
|
||||
if (!(isRecipientAlsoMinter && existingRewardShareData == null))
|
||||
// Check creator has enough funds
|
||||
if (creator.getConfirmedBalance(Asset.QORT).compareTo(rewardShareTransactionData.getFee()) < 0)
|
||||
return ValidationResult.NO_BALANCE;
|
||||
}
|
||||
|
||||
return ValidationResult.OK;
|
||||
}
|
||||
|
@@ -314,6 +314,7 @@ public abstract class Transaction {
|
||||
return Transaction.getDeadline(transactionData);
|
||||
}
|
||||
|
||||
/** Returns whether transaction's fee is at least minimum unit fee as specified in blockchain config. */
|
||||
public boolean hasMinimumFee() {
|
||||
return this.transactionData.getFee().compareTo(BlockChain.getInstance().getUnitFee()) >= 0;
|
||||
}
|
||||
@@ -326,6 +327,7 @@ public abstract class Transaction {
|
||||
}
|
||||
}
|
||||
|
||||
/** Returns whether transaction's fee is at least amount needed to cover byte-length of transaction. */
|
||||
public boolean hasMinimumFeePerByte() {
|
||||
return this.feePerByte().compareTo(BlockChain.getInstance().getMinFeePerByte()) >= 0;
|
||||
}
|
||||
@@ -538,8 +540,9 @@ public abstract class Transaction {
|
||||
return ValidationResult.TIMESTAMP_TOO_NEW;
|
||||
|
||||
// Check fee is sufficient
|
||||
if (!hasMinimumFee() || !hasMinimumFeePerByte())
|
||||
return ValidationResult.INSUFFICIENT_FEE;
|
||||
ValidationResult feeValidationResult = isFeeValid();
|
||||
if (feeValidationResult != ValidationResult.OK)
|
||||
return feeValidationResult;
|
||||
|
||||
/*
|
||||
* We have to grab the blockchain lock because we're updating
|
||||
@@ -591,6 +594,14 @@ public abstract class Transaction {
|
||||
}
|
||||
}
|
||||
|
||||
/** Returns whether transaction's fee is valid. Might be overriden in transaction subclasses. */
|
||||
protected ValidationResult isFeeValid() throws DataException {
|
||||
if (!hasMinimumFee() || !hasMinimumFeePerByte())
|
||||
return ValidationResult.INSUFFICIENT_FEE;
|
||||
|
||||
return ValidationResult.OK;
|
||||
}
|
||||
|
||||
private boolean isValidTxGroupId() throws DataException {
|
||||
int txGroupId = this.transactionData.getTxGroupId();
|
||||
|
||||
|
Reference in New Issue
Block a user