diff --git a/src/main/java/org/qortal/repository/GroupRepository.java b/src/main/java/org/qortal/repository/GroupRepository.java index bcee7d25..94c97992 100644 --- a/src/main/java/org/qortal/repository/GroupRepository.java +++ b/src/main/java/org/qortal/repository/GroupRepository.java @@ -131,7 +131,14 @@ public interface GroupRepository { public GroupBanData getBan(int groupId, String member) throws DataException; - public boolean banExists(int groupId, String offender) throws DataException; + /** + * IMPORTANT: when using banExists() as part of validation, the timestamp must be that of the transaction that + * is calling banExists() as part of its validation. It must NOT be the current time, unless this is being + * called outside of validation, as part of an on demand check for a ban existing (such as via an API call). + * This is because we need to evaluate a ban's status based on the time of the subsequent transaction, as + * validation will not occur at a fixed time for every node. For some, it could be months into the future. + */ + public boolean banExists(int groupId, String offender, long timestamp) throws DataException; public List getGroupBans(int groupId, Integer limit, Integer offset, Boolean reverse) throws DataException; diff --git a/src/main/java/org/qortal/repository/hsqldb/HSQLDBGroupRepository.java b/src/main/java/org/qortal/repository/hsqldb/HSQLDBGroupRepository.java index 91db22f1..b1cd40a0 100644 --- a/src/main/java/org/qortal/repository/hsqldb/HSQLDBGroupRepository.java +++ b/src/main/java/org/qortal/repository/hsqldb/HSQLDBGroupRepository.java @@ -777,9 +777,9 @@ public class HSQLDBGroupRepository implements GroupRepository { } @Override - public boolean banExists(int groupId, String offender) throws DataException { + public boolean banExists(int groupId, String offender, long timestamp) throws DataException { try { - return this.repository.exists("GroupBans", "group_id = ? AND offender = ?", groupId, offender); + return this.repository.exists("GroupBans", "group_id = ? AND offender = ? AND (expires_when IS NULL OR expires_when > ?)", groupId, offender, timestamp); } catch (SQLException e) { throw new DataException("Unable to check for group ban in repository", e); } diff --git a/src/main/java/org/qortal/transaction/CancelGroupBanTransaction.java b/src/main/java/org/qortal/transaction/CancelGroupBanTransaction.java index 483dfc6f..08d9cb3e 100644 --- a/src/main/java/org/qortal/transaction/CancelGroupBanTransaction.java +++ b/src/main/java/org/qortal/transaction/CancelGroupBanTransaction.java @@ -73,7 +73,7 @@ public class CancelGroupBanTransaction extends Transaction { Account member = getMember(); // Check ban actually exists - if (!this.repository.getGroupRepository().banExists(groupId, member.getAddress())) + if (!this.repository.getGroupRepository().banExists(groupId, member.getAddress(), this.groupUnbanTransactionData.getTimestamp())) return ValidationResult.BAN_UNKNOWN; // Check admin has enough funds diff --git a/src/main/java/org/qortal/transaction/GroupInviteTransaction.java b/src/main/java/org/qortal/transaction/GroupInviteTransaction.java index f3b08f59..fa5e7b85 100644 --- a/src/main/java/org/qortal/transaction/GroupInviteTransaction.java +++ b/src/main/java/org/qortal/transaction/GroupInviteTransaction.java @@ -78,7 +78,7 @@ public class GroupInviteTransaction extends Transaction { return ValidationResult.ALREADY_GROUP_MEMBER; // Check invitee is not banned - if (this.repository.getGroupRepository().banExists(groupId, invitee.getAddress())) + if (this.repository.getGroupRepository().banExists(groupId, invitee.getAddress(), this.groupInviteTransactionData.getTimestamp())) return ValidationResult.BANNED_FROM_GROUP; // Check creator has enough funds diff --git a/src/main/java/org/qortal/transaction/JoinGroupTransaction.java b/src/main/java/org/qortal/transaction/JoinGroupTransaction.java index bc62c629..3061a3fb 100644 --- a/src/main/java/org/qortal/transaction/JoinGroupTransaction.java +++ b/src/main/java/org/qortal/transaction/JoinGroupTransaction.java @@ -53,7 +53,7 @@ public class JoinGroupTransaction extends Transaction { return ValidationResult.ALREADY_GROUP_MEMBER; // Check member is not banned - if (this.repository.getGroupRepository().banExists(groupId, joiner.getAddress())) + if (this.repository.getGroupRepository().banExists(groupId, joiner.getAddress(), this.joinGroupTransactionData.getTimestamp())) return ValidationResult.BANNED_FROM_GROUP; // Check join request doesn't already exist diff --git a/src/main/java/org/qortal/transaction/UpdateGroupTransaction.java b/src/main/java/org/qortal/transaction/UpdateGroupTransaction.java index 9664ccbf..27580430 100644 --- a/src/main/java/org/qortal/transaction/UpdateGroupTransaction.java +++ b/src/main/java/org/qortal/transaction/UpdateGroupTransaction.java @@ -103,7 +103,7 @@ public class UpdateGroupTransaction extends Transaction { Account newOwner = getNewOwner(); // Check new owner is not banned - if (this.repository.getGroupRepository().banExists(this.updateGroupTransactionData.getGroupId(), newOwner.getAddress())) + if (this.repository.getGroupRepository().banExists(this.updateGroupTransactionData.getGroupId(), newOwner.getAddress(), this.updateGroupTransactionData.getTimestamp())) return ValidationResult.BANNED_FROM_GROUP; return ValidationResult.OK; diff --git a/src/test/java/org/qortal/test/group/AdminTests.java b/src/test/java/org/qortal/test/group/AdminTests.java index a39b23d7..8cf83c29 100644 --- a/src/test/java/org/qortal/test/group/AdminTests.java +++ b/src/test/java/org/qortal/test/group/AdminTests.java @@ -135,7 +135,8 @@ public class AdminTests extends Common { assertNotSame(ValidationResult.OK, result); // Attempt to ban Bob - result = groupBan(repository, alice, groupId, bob.getAddress()); + int timeToLive = 0; + result = groupBan(repository, alice, groupId, bob.getAddress(), timeToLive); // Should be OK assertEquals(ValidationResult.OK, result); @@ -158,7 +159,7 @@ public class AdminTests extends Common { assertTrue(isMember(repository, bob.getAddress(), groupId)); // Attempt to ban Bob - result = groupBan(repository, alice, groupId, bob.getAddress()); + result = groupBan(repository, alice, groupId, bob.getAddress(), timeToLive); // Should be OK assertEquals(ValidationResult.OK, result); @@ -205,6 +206,144 @@ public class AdminTests extends Common { } } + @Test + public void testGroupBanMemberWithExpiry() throws DataException, InterruptedException { + try (final Repository repository = RepositoryManager.getRepository()) { + PrivateKeyAccount alice = Common.getTestAccount(repository, "alice"); + PrivateKeyAccount bob = Common.getTestAccount(repository, "bob"); + + // Create group + int groupId = createGroup(repository, alice, "open-group", true); + + // Confirm Bob is not a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Attempt to cancel non-existent Bob ban + ValidationResult result = cancelGroupBan(repository, alice, groupId, bob.getAddress()); + // Should NOT be OK + assertNotSame(ValidationResult.OK, result); + + // Attempt to ban Bob for 2 seconds + int timeToLive = 2; + result = groupBan(repository, alice, groupId, bob.getAddress(), timeToLive); + // Should be OK + assertEquals(ValidationResult.OK, result); + + // Confirm Bob no longer a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Bob attempts to rejoin + result = joinGroup(repository, bob, groupId); + // Should NOT be OK + assertNotSame(ValidationResult.OK, result); + + // Wait for 2 seconds to pass + Thread.sleep(2000L); + + // Bob attempts to rejoin again + result = joinGroup(repository, bob, groupId); + // Should be OK, as the ban has expired + assertSame(ValidationResult.OK, result); + + // Confirm Bob is now a member + assertTrue(isMember(repository, bob.getAddress(), groupId)); + + // Orphan last block (Bob join) + BlockUtils.orphanLastBlock(repository); + + // Confirm Bob is not a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Orphan last block (Bob ban) + BlockUtils.orphanLastBlock(repository); + // Delete unconfirmed group-ban transaction + TransactionUtils.deleteUnconfirmedTransactions(repository); + + // Bob to join + result = joinGroup(repository, bob, groupId); + // Should be OK + assertEquals(ValidationResult.OK, result); + + // Confirm Bob now a member + assertTrue(isMember(repository, bob.getAddress(), groupId)); + + + // Attempt to ban Bob for 2 seconds + result = groupBan(repository, alice, groupId, bob.getAddress(), 2); + // Should be OK + assertEquals(ValidationResult.OK, result); + + // Confirm Bob no longer a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Wait for 2 seconds to pass + Thread.sleep(2000L); + + // Cancel Bob's ban + result = cancelGroupBan(repository, alice, groupId, bob.getAddress()); + // Should NOT be OK, as ban has already expired + assertNotSame(ValidationResult.OK, result); + + // Confirm Bob still not a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Bob attempts to rejoin + result = joinGroup(repository, bob, groupId); + // Should be OK, as no longer banned + assertSame(ValidationResult.OK, result); + + // Confirm Bob is now a member + assertTrue(isMember(repository, bob.getAddress(), groupId)); + + + // Attempt to ban Bob for 10 seconds + result = groupBan(repository, alice, groupId, bob.getAddress(), 10); + // Should be OK + assertEquals(ValidationResult.OK, result); + + // Confirm Bob no longer a member + assertFalse(isMember(repository, bob.getAddress(), groupId)); + + // Bob attempts to rejoin + result = joinGroup(repository, bob, groupId); + // Should NOT be OK, as ban still exists + assertNotSame(ValidationResult.OK, result); + + // Cancel Bob's ban + result = cancelGroupBan(repository, alice, groupId, bob.getAddress()); + // Should be OK, as ban still exists + assertEquals(ValidationResult.OK, result); + + // Bob attempts to rejoin + result = joinGroup(repository, bob, groupId); + // Should be OK, as no longer banned + assertEquals(ValidationResult.OK, result); + + // Orphan last block (Bob join) + BlockUtils.orphanLastBlock(repository); + // Delete unconfirmed join-group transaction + TransactionUtils.deleteUnconfirmedTransactions(repository); + + // Orphan last block (Cancel Bob ban) + BlockUtils.orphanLastBlock(repository); + // Delete unconfirmed cancel-ban transaction + TransactionUtils.deleteUnconfirmedTransactions(repository); + + // Bob attempts to rejoin + result = joinGroup(repository, bob, groupId); + // Should NOT be OK + assertNotSame(ValidationResult.OK, result); + + // Orphan last block (Bob ban) + BlockUtils.orphanLastBlock(repository); + // Delete unconfirmed group-ban transaction + TransactionUtils.deleteUnconfirmedTransactions(repository); + + // Confirm Bob now a member + assertTrue(isMember(repository, bob.getAddress(), groupId)); + } + } + @Test public void testGroupBanAdmin() throws DataException { try (final Repository repository = RepositoryManager.getRepository()) { @@ -226,7 +365,8 @@ public class AdminTests extends Common { assertTrue(isAdmin(repository, bob.getAddress(), groupId)); // Attempt to ban Bob - result = groupBan(repository, alice, groupId, bob.getAddress()); + int timeToLive = 0; + result = groupBan(repository, alice, groupId, bob.getAddress(), timeToLive); // Should be OK assertEquals(ValidationResult.OK, result); @@ -272,12 +412,12 @@ public class AdminTests extends Common { assertTrue(isAdmin(repository, bob.getAddress(), groupId)); // Have Alice (owner) try to ban herself! - result = groupBan(repository, alice, groupId, alice.getAddress()); + result = groupBan(repository, alice, groupId, alice.getAddress(), timeToLive); // Should NOT be OK assertNotSame(ValidationResult.OK, result); // Have Bob try to ban Alice (owner) - result = groupBan(repository, bob, groupId, alice.getAddress()); + result = groupBan(repository, bob, groupId, alice.getAddress(), timeToLive); // Should NOT be OK assertNotSame(ValidationResult.OK, result); } @@ -316,8 +456,8 @@ public class AdminTests extends Common { return result; } - private ValidationResult groupBan(Repository repository, PrivateKeyAccount admin, int groupId, String member) throws DataException { - GroupBanTransactionData transactionData = new GroupBanTransactionData(TestTransaction.generateBase(admin), groupId, member, "testing", 0); + private ValidationResult groupBan(Repository repository, PrivateKeyAccount admin, int groupId, String member, int timeToLive) throws DataException { + GroupBanTransactionData transactionData = new GroupBanTransactionData(TestTransaction.generateBase(admin), groupId, member, "testing", timeToLive); ValidationResult result = TransactionUtils.signAndImport(repository, transactionData, admin); if (result == ValidationResult.OK)