Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minter Group Check Optimizations #221

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Minter Group Check Optimizations #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
130bb6c
Added logging statements to demonstrate order of operations. This wil…
kennycud Nov 18, 2024
f55efe3
Removed logging statements to demonstrate order of operations to othe…
kennycud Nov 18, 2024
f277611
Merge branch 'master' of https://github.com/kennycud/qortal
kennycud Nov 20, 2024
b0d43a1
minter group check optimizations
kennycud Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/main/java/org/qortal/account/Account.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,11 @@ public boolean isFounder() throws DataException {
* <li>account's address is a member of the minter group</li>
* </ul>
*
* @param isGroupValidated true if this account has already been validated for MINTER Group membership
* @return true if account can be considered "minting account"
* @throws DataException
*/
public boolean canMint() throws DataException {
public boolean canMint(boolean isGroupValidated) throws DataException {
AccountData accountData = this.repository.getAccountRepository().getAccount(this.address);
NameRepository nameRepository = this.repository.getNameRepository();
GroupRepository groupRepository = this.repository.getGroupRepository();
Expand Down Expand Up @@ -251,9 +252,9 @@ public boolean canMint() throws DataException {
if (blockchainHeight >= groupCheckHeight && blockchainHeight < removeNameCheckHeight) {
List<NameData> myName = nameRepository.getNamesByOwner(myAddress);
if (Account.isFounder(accountData.getFlags())) {
return accountData.getBlocksMintedPenalty() == 0 && !myName.isEmpty() && groupRepository.memberExists(groupIdToMint, myAddress);
return accountData.getBlocksMintedPenalty() == 0 && !myName.isEmpty() && (isGroupValidated || groupRepository.memberExists(groupIdToMint, myAddress));
} else {
return level >= levelToMint && !myName.isEmpty() && groupRepository.memberExists(groupIdToMint, myAddress);
return level >= levelToMint && !myName.isEmpty() && (isGroupValidated || groupRepository.memberExists(groupIdToMint, myAddress));
}
}

Expand All @@ -262,9 +263,9 @@ public boolean canMint() throws DataException {
// Account's address is a member of the minter group
if (blockchainHeight >= removeNameCheckHeight) {
if (Account.isFounder(accountData.getFlags())) {
return accountData.getBlocksMintedPenalty() == 0 && groupRepository.memberExists(groupIdToMint, myAddress);
return accountData.getBlocksMintedPenalty() == 0 && (isGroupValidated || groupRepository.memberExists(groupIdToMint, myAddress));
} else {
return level >= levelToMint && groupRepository.memberExists(groupIdToMint, myAddress);
return level >= levelToMint && (isGroupValidated || groupRepository.memberExists(groupIdToMint, myAddress));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/qortal/api/restricted/resource/AdminResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ public String addMintingAccount(@HeaderParam(Security.API_KEY_HEADER) String api

// Qortal: check reward-share's minting account is still allowed to mint
Account rewardShareMintingAccount = new Account(repository, rewardShareData.getMinter());
if (!rewardShareMintingAccount.canMint())
if (!rewardShareMintingAccount.canMint(false))
throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.CANNOT_MINT);

MintingAccountData mintingAccountData = new MintingAccountData(mintingAccount.getPrivateKey(), mintingAccount.getPublicKey());
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/qortal/block/Block.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1518,7 +1518,7 @@ protected boolean isMinterValid(Block parentBlock) throws DataException {
return false;

Account mintingAccount = new PublicKeyAccount(this.repository, rewardShareData.getMinterPublicKey());
return mintingAccount.canMint();
return mintingAccount.canMint(false);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/qortal/controller/BlockMinter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void run() {
}

Account mintingAccount = new Account(repository, rewardShareData.getMinter());
if (!mintingAccount.canMint()) {
if (!mintingAccount.canMint(true)) {
// Minting-account component of reward-share can no longer mint - disregard
madi.remove();
continue;
Expand Down
20 changes: 16 additions & 4 deletions src/main/java/org/qortal/controller/OnlineAccountsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.qortal.crypto.Qortal25519Extras;
import org.qortal.data.account.MintingAccountData;
import org.qortal.data.account.RewardShareData;
import org.qortal.data.group.GroupMemberData;
import org.qortal.data.network.OnlineAccountData;
import org.qortal.network.Network;
import org.qortal.network.Peer;
Expand Down Expand Up @@ -224,6 +225,12 @@ private void processOnlineAccountsImportQueue() {
Set<OnlineAccountData> onlineAccountsToAdd = new HashSet<>();
Set<OnlineAccountData> onlineAccountsToRemove = new HashSet<>();
try (final Repository repository = RepositoryManager.getRepository()) {
List<String> mintingGroupMemberAddresses
= repository.getGroupRepository()
.getGroupMembers(BlockChain.getInstance().getMintingGroupId()).stream()
.map(GroupMemberData::getMember)
.collect(Collectors.toList());

for (OnlineAccountData onlineAccountData : this.onlineAccountsImportQueue) {
if (isStopping)
return;
Expand All @@ -236,7 +243,7 @@ private void processOnlineAccountsImportQueue() {
continue;
}

boolean isValid = this.isValidCurrentAccount(repository, onlineAccountData);
boolean isValid = this.isValidCurrentAccount(repository, mintingGroupMemberAddresses, onlineAccountData);
if (isValid)
onlineAccountsToAdd.add(onlineAccountData);

Expand Down Expand Up @@ -315,7 +322,7 @@ public static byte[] xorByteArrayInPlace(byte[] inplaceArray, byte[] otherArray)
return inplaceArray;
}

private static boolean isValidCurrentAccount(Repository repository, OnlineAccountData onlineAccountData) throws DataException {
private static boolean isValidCurrentAccount(Repository repository, List<String> mintingGroupMemberAddresses, OnlineAccountData onlineAccountData) throws DataException {
final Long now = NTP.getTime();
if (now == null)
return false;
Expand Down Expand Up @@ -350,9 +357,14 @@ private static boolean isValidCurrentAccount(Repository repository, OnlineAccoun
LOGGER.trace(() -> String.format("Rejecting unknown online reward-share public key %s", Base58.encode(rewardSharePublicKey)));
return false;
}
// reject account address that are not in the MINTER Group
else if( !mintingGroupMemberAddresses.contains(rewardShareData.getMinter())) {
LOGGER.trace(() -> String.format("Rejecting online reward-share that is not in MINTER Group, account %s", rewardShareData.getMinter()));
return false;
}

Account mintingAccount = new Account(repository, rewardShareData.getMinter());
if (!mintingAccount.canMint()) {
if (!mintingAccount.canMint(true)) { // group validation is a few lines above
// Minting-account component of reward-share can no longer mint - disregard
LOGGER.trace(() -> String.format("Rejecting online reward-share with non-minting account %s", mintingAccount.getAddress()));
return false;
Expand Down Expand Up @@ -539,7 +551,7 @@ private boolean computeOurAccountsForTimestamp(Long onlineAccountsTimestamp) {
}

Account mintingAccount = new Account(repository, rewardShareData.getMinter());
if (!mintingAccount.canMint()) {
if (!mintingAccount.canMint(true)) {
// Minting-account component of reward-share can no longer mint - disregard
iterator.remove();
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/qortal/transaction/RewardShareTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public ValidationResult isValid() throws DataException {
final boolean isCancellingSharePercent = this.rewardShareTransactionData.getSharePercent() < 0;

// Creator themselves needs to be allowed to mint (unless cancelling)
if (!isCancellingSharePercent && !creator.canMint())
if (!isCancellingSharePercent && !creator.canMint(false))
return ValidationResult.NOT_MINTING_ACCOUNT;

// Qortal: special rules in play depending whether recipient is also minter
Expand Down
54 changes: 27 additions & 27 deletions src/test/java/org/qortal/test/TransferPrivsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void testNewAccountsTransferPrivs() throws DataException {
public void testAliceIntoNewAccountTransferPrivs() throws DataException {
try (final Repository repository = RepositoryManager.getRepository()) {
TestAccount alice = Common.getTestAccount(repository, "alice");
assertTrue(alice.canMint());
assertTrue(alice.canMint(false));

PrivateKeyAccount aliceMintingAccount = Common.getTestAccount(repository, "alice-reward-share");

Expand All @@ -86,8 +86,8 @@ public void testAliceIntoNewAccountTransferPrivs() throws DataException {

combineAccounts(repository, alice, randomAccount, aliceMintingAccount);

assertFalse(alice.canMint());
assertTrue(randomAccount.canMint());
assertFalse(alice.canMint(false));
assertTrue(randomAccount.canMint(false));
}
}

Expand All @@ -97,8 +97,8 @@ public void testAliceIntoDilbertTransferPrivs() throws DataException {
TestAccount alice = Common.getTestAccount(repository, "alice");
TestAccount dilbert = Common.getTestAccount(repository, "dilbert");

assertTrue(alice.canMint());
assertTrue(dilbert.canMint());
assertTrue(alice.canMint(false));
assertTrue(dilbert.canMint(false));

// Dilbert has level, Alice does not so we need Alice to mint enough blocks to bump Dilbert's level post-combine
final int expectedPostCombineLevel = dilbert.getLevel() + 1;
Expand All @@ -118,24 +118,24 @@ public void testAliceIntoDilbertTransferPrivs() throws DataException {

// Post-combine sender checks
checkSenderPostTransfer(postCombineAliceData);
assertFalse(alice.canMint());
assertFalse(alice.canMint(false));

// Post-combine recipient checks
checkRecipientPostTransfer(preCombineAliceData, preCombineDilbertData, postCombineDilbertData, expectedPostCombineLevel);
assertTrue(dilbert.canMint());
assertTrue(dilbert.canMint(false));

// Orphan previous block
BlockUtils.orphanLastBlock(repository);

// Sender checks
AccountData orphanedAliceData = repository.getAccountRepository().getAccount(alice.getAddress());
checkAccountDataRestored("sender", preCombineAliceData, orphanedAliceData);
assertTrue(alice.canMint());
assertTrue(alice.canMint(false));

// Recipient checks
AccountData orphanedDilbertData = repository.getAccountRepository().getAccount(dilbert.getAddress());
checkAccountDataRestored("recipient", preCombineDilbertData, orphanedDilbertData);
assertTrue(dilbert.canMint());
assertTrue(dilbert.canMint(false));
}
}

Expand All @@ -145,8 +145,8 @@ public void testDilbertIntoAliceTransferPrivs() throws DataException {
TestAccount alice = Common.getTestAccount(repository, "alice");
TestAccount dilbert = Common.getTestAccount(repository, "dilbert");

assertTrue(dilbert.canMint());
assertTrue(alice.canMint());
assertTrue(dilbert.canMint(false));
assertTrue(alice.canMint(false));

// Dilbert has level, Alice does not so we need Alice to mint enough blocks to surpass Dilbert's level post-combine
final int expectedPostCombineLevel = dilbert.getLevel() + 1;
Expand All @@ -166,24 +166,24 @@ public void testDilbertIntoAliceTransferPrivs() throws DataException {

// Post-combine sender checks
checkSenderPostTransfer(postCombineDilbertData);
assertFalse(dilbert.canMint());
assertFalse(dilbert.canMint(false));

// Post-combine recipient checks
checkRecipientPostTransfer(preCombineDilbertData, preCombineAliceData, postCombineAliceData, expectedPostCombineLevel);
assertTrue(alice.canMint());
assertTrue(alice.canMint(false));

// Orphan previous block
BlockUtils.orphanLastBlock(repository);

// Sender checks
AccountData orphanedDilbertData = repository.getAccountRepository().getAccount(dilbert.getAddress());
checkAccountDataRestored("sender", preCombineDilbertData, orphanedDilbertData);
assertTrue(dilbert.canMint());
assertTrue(dilbert.canMint(false));

// Recipient checks
AccountData orphanedAliceData = repository.getAccountRepository().getAccount(alice.getAddress());
checkAccountDataRestored("recipient", preCombineAliceData, orphanedAliceData);
assertTrue(alice.canMint());
assertTrue(alice.canMint(false));
}
}

Expand All @@ -202,8 +202,8 @@ public void testMultipleIntoChloeTransferPrivs() throws DataException {
TestAccount chloe = Common.getTestAccount(repository, "chloe");
TestAccount dilbert = Common.getTestAccount(repository, "dilbert");

assertTrue(dilbert.canMint());
assertFalse(chloe.canMint());
assertTrue(dilbert.canMint(false));
assertFalse(chloe.canMint(false));

// COMBINE DILBERT INTO CHLOE

Expand All @@ -225,16 +225,16 @@ public void testMultipleIntoChloeTransferPrivs() throws DataException {

// Post-combine sender checks
checkSenderPostTransfer(post1stCombineDilbertData);
assertFalse(dilbert.canMint());
assertFalse(dilbert.canMint(false));

// Post-combine recipient checks
checkRecipientPostTransfer(pre1stCombineDilbertData, pre1stCombineChloeData, post1stCombineChloeData, expectedPost1stCombineLevel);
assertTrue(chloe.canMint());
assertTrue(chloe.canMint(false));

// COMBINE ALICE INTO CHLOE

assertTrue(alice.canMint());
assertTrue(chloe.canMint());
assertTrue(alice.canMint(false));
assertTrue(chloe.canMint(false));

// Alice needs to mint enough blocks to surpass Chloe's level post-combine
final int expectedPost2ndCombineLevel = chloe.getLevel() + 1;
Expand All @@ -254,40 +254,40 @@ public void testMultipleIntoChloeTransferPrivs() throws DataException {

// Post-combine sender checks
checkSenderPostTransfer(post2ndCombineAliceData);
assertFalse(alice.canMint());
assertFalse(alice.canMint(false));

// Post-combine recipient checks
checkRecipientPostTransfer(pre2ndCombineAliceData, pre2ndCombineChloeData, post2ndCombineChloeData, expectedPost2ndCombineLevel);
assertTrue(chloe.canMint());
assertTrue(chloe.canMint(false));

// Orphan 2nd combine
BlockUtils.orphanLastBlock(repository);

// Sender checks
AccountData orphanedAliceData = repository.getAccountRepository().getAccount(alice.getAddress());
checkAccountDataRestored("sender", pre2ndCombineAliceData, orphanedAliceData);
assertTrue(alice.canMint());
assertTrue(alice.canMint(false));

// Recipient checks
AccountData orphanedChloeData = repository.getAccountRepository().getAccount(chloe.getAddress());
checkAccountDataRestored("recipient", pre2ndCombineChloeData, orphanedChloeData);
assertTrue(chloe.canMint());
assertTrue(chloe.canMint(false));

// Orphan 1nd combine
BlockUtils.orphanToBlock(repository, pre1stCombineBlockHeight);

// Sender checks
AccountData orphanedDilbertData = repository.getAccountRepository().getAccount(dilbert.getAddress());
checkAccountDataRestored("sender", pre1stCombineDilbertData, orphanedDilbertData);
assertTrue(dilbert.canMint());
assertTrue(dilbert.canMint(false));

// Recipient checks
orphanedChloeData = repository.getAccountRepository().getAccount(chloe.getAddress());
checkAccountDataRestored("recipient", pre1stCombineChloeData, orphanedChloeData);

// Chloe canMint() would return true here due to Alice-Chloe reward-share minting at top of method, so undo that minting by orphaning back to block 1
BlockUtils.orphanToBlock(repository, 1);
assertFalse(chloe.canMint());
assertFalse(chloe.canMint(false));
}
}

Expand Down
Loading