Fixed group ban expiry.

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<GroupBanData> getGroupBans(int groupId, Integer limit, Integer offset, Boolean reverse) throws DataException;
 

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);
 		}

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

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

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

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;

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)