Skip to content

Commit

Permalink
Merge pull request #2912 from rsksmart/whitelistSupport-refactor
Browse files Browse the repository at this point in the history 
feat: changed method getLockWhitelistEntry so it returns an optional
  • Loading branch information
@marcos-iov
marcos-iov authored Jan 14, 2025
2 parents c5e3dc4 + d4e78c6 commit 494c619
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 63 deletions.
31 changes: 15 additions & 16 deletions rskj-core/src/main/java/co/rsk/peg/Bridge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1037,14 +1037,11 @@ public String getLockWhitelistAddress(Object[] args) {
logger.trace("getLockWhitelistAddress");

int index = ((BigInteger) args[0]).intValue();
LockWhitelistEntry entry = bridgeSupport.getLockWhitelistEntryByIndex(index);
Optional<LockWhitelistEntry> entry = bridgeSupport.getLockWhitelistEntryByIndex(index);

if (entry == null) {
// Empty string is returned when address is not found
return "";
}
// Empty string is returned when address is not found
return entry.map(lockWhitelistEntry -> lockWhitelistEntry.address().toBase58()).orElse("");

return entry.address().toBase58();
}

public long getLockWhitelistEntryByAddress(Object[] args) {
Expand All @@ -1058,17 +1055,19 @@ public long getLockWhitelistEntryByAddress(Object[] args) {
return WhitelistResponseCode.INVALID_ADDRESS_FORMAT.getCode();
}

LockWhitelistEntry entry = bridgeSupport.getLockWhitelistEntryByAddress(addressBase58);

if (entry == null) {
// Empty string is returned when address is not found
logger.debug("[getLockWhitelistEntryByAddress] Address not found: {}", addressBase58);
return WhitelistResponseCode.ADDRESS_NOT_EXIST.getCode();
}
return bridgeSupport.getLockWhitelistEntryByAddress(addressBase58)
.map(lockWhitelistEntry -> {
if (lockWhitelistEntry.getClass() == OneOffWhiteListEntry.class) {
return ((OneOffWhiteListEntry) lockWhitelistEntry).maxTransferValue().getValue();
}

return entry.getClass() == OneOffWhiteListEntry.class ?
((OneOffWhiteListEntry)entry).maxTransferValue().getValue() :
WhitelistResponseCode.UNLIMITED_MODE.getCode();
return WhitelistResponseCode.UNLIMITED_MODE.getCode();
})
.orElseGet(() -> {
// Empty string is returned when address is not found
logger.debug("[getLockWhitelistEntryByAddress] Address not found: {}", addressBase58);
return WhitelistResponseCode.ADDRESS_NOT_EXIST.getCode();
}).longValue();
}

public Integer addOneOffLockWhitelistAddress(Object[] args) {
Expand Down
4 changes: 2 additions & 2 deletions rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2121,11 +2121,11 @@ public Integer getLockWhitelistSize() {
return whitelistSupport.getLockWhitelistSize();
}

public LockWhitelistEntry getLockWhitelistEntryByIndex(int index) {
public Optional<LockWhitelistEntry> getLockWhitelistEntryByIndex(int index) {
return whitelistSupport.getLockWhitelistEntryByIndex(index);
}

public LockWhitelistEntry getLockWhitelistEntryByAddress(String addressBase58) {
public Optional<LockWhitelistEntry> getLockWhitelistEntryByAddress(String addressBase58) {
return whitelistSupport.getLockWhitelistEntryByAddress(addressBase58);
}

Expand Down
14 changes: 8 additions & 6 deletions rskj-core/src/main/java/co/rsk/peg/whitelist/WhitelistSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import co.rsk.bitcoinj.core.Address;
import co.rsk.bitcoinj.core.Coin;
import java.math.BigInteger;
import java.util.Optional;

import org.ethereum.core.Transaction;

/**
Expand All @@ -19,22 +21,22 @@ public interface WhitelistSupport {
int getLockWhitelistSize();

/**
* Returns the lock whitelist entry stored at the given index, or null if the index is out of
* Returns the lock whitelist entry stored at the given index, or an empty Optional if the index is out of
* bounds
*
* @param index the index at which to get the entry
* @return the whitelist entry stored at the given index, or null if index is out of
* @return the whitelist entry stored at the given index, or an empty Optional if index is out of
* bounds
*/
LockWhitelistEntry getLockWhitelistEntryByIndex(int index);
Optional<LockWhitelistEntry> getLockWhitelistEntryByIndex(int index);

/**
* Returns the lock whitelist entry for a given address, or null if the address is not whitelisted
* Returns the lock whitelist entry for a given address, or an empty Optional if the address is not whitelisted
*
* @param addressBase58 the address in base58 format to search for
* @return the whitelist entry for the given address, or null if the addrres is not whitelisted
* @return the whitelist entry for the given address, or an empty Optional if the addrres is not whitelisted
*/
LockWhitelistEntry getLockWhitelistEntryByAddress(String addressBase58);
Optional<LockWhitelistEntry> getLockWhitelistEntryByAddress(String addressBase58);

/**
* Adds the given address to the lock whitelist, allowing peg-ins up to certain max value
Expand Down
14 changes: 8 additions & 6 deletions rskj-core/src/main/java/co/rsk/peg/whitelist/WhitelistSupportImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import co.rsk.peg.whitelist.constants.WhitelistConstants;
import java.math.BigInteger;
import java.util.List;
import java.util.Optional;

import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.core.SignatureCache;
import org.ethereum.core.Transaction;
Expand Down Expand Up @@ -43,27 +45,27 @@ public int getLockWhitelistSize() {
}

@Override
public LockWhitelistEntry getLockWhitelistEntryByIndex(int index) {
public Optional<LockWhitelistEntry> getLockWhitelistEntryByIndex(int index) {
List<LockWhitelistEntry> entries = storageProvider.getLockWhitelist(
activations,
networkParameters
).getAll();

if (index < 0 || index >= entries.size()) {
return null;
return Optional.empty();
}
return entries.get(index);
return Optional.ofNullable(entries.get(index));
}

@Override
public LockWhitelistEntry getLockWhitelistEntryByAddress(String addressBase58) {
public Optional<LockWhitelistEntry> getLockWhitelistEntryByAddress(String addressBase58) {
try {
Address address = Address.fromBase58(networkParameters, addressBase58);

return storageProvider.getLockWhitelist(activations, networkParameters).get(address);
return Optional.ofNullable(storageProvider.getLockWhitelist(activations, networkParameters).get(address));
} catch (AddressFormatException e) {
logger.warn("[getLockWhitelistEntryByAddress] {}", INVALID_ADDRESS_FORMAT_MESSAGE, e);
return null;
return Optional.empty();
}
}

Expand Down
13 changes: 9 additions & 4 deletions rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.core.Is.is;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -202,17 +203,21 @@ void getLockWhitelistSize() {
@Test
void getLockWhitelistEntryByIndex() {
LockWhitelistEntry entry = mock(LockWhitelistEntry.class);
when(whitelistSupport.getLockWhitelistEntryByIndex(0)).thenReturn(entry);
when(whitelistSupport.getLockWhitelistEntryByIndex(0)).thenReturn(Optional.of(entry));

assertEquals(entry, bridgeSupport.getLockWhitelistEntryByIndex(0));
Optional<LockWhitelistEntry> lockWhitelistEntryByIndex = bridgeSupport.getLockWhitelistEntryByIndex(0);
assertTrue(lockWhitelistEntryByIndex.isPresent());
assertEquals(entry, lockWhitelistEntryByIndex.get());
}

@Test
void getLockWhitelistEntryByAddress() {
LockWhitelistEntry entry = mock(LockWhitelistEntry.class);
when(whitelistSupport.getLockWhitelistEntryByAddress("address")).thenReturn(entry);
when(whitelistSupport.getLockWhitelistEntryByAddress("address")).thenReturn(Optional.of(entry));

assertEquals(entry, bridgeSupport.getLockWhitelistEntryByAddress("address"));
Optional<LockWhitelistEntry> lockWhitelistEntry = bridgeSupport.getLockWhitelistEntryByAddress("address");
assertTrue(lockWhitelistEntry.isPresent());
assertEquals(entry, lockWhitelistEntry.get());
}

@Test
Expand Down
8 changes: 4 additions & 4 deletions rskj-core/src/test/java/co/rsk/peg/BridgeTestIntegration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2180,8 +2180,8 @@ void getLockWhitelistAddress() {
OneOffWhiteListEntry mockedEntry10 = new OneOffWhiteListEntry(new BtcECKey().toAddress(networkParameters), Coin.COIN);
OneOffWhiteListEntry mockedEntry20 = new OneOffWhiteListEntry(new BtcECKey().toAddress(networkParameters), Coin.COIN);
TestUtils.setInternalState(bridge, "bridgeSupport", bridgeSupportMock);
when(bridgeSupportMock.getLockWhitelistEntryByIndex(10)).then((InvocationOnMock invocation) -> mockedEntry10);
when(bridgeSupportMock.getLockWhitelistEntryByIndex(20)).then((InvocationOnMock invocation) -> mockedEntry20);
when(bridgeSupportMock.getLockWhitelistEntryByIndex(10)).then((InvocationOnMock invocation) -> Optional.of(mockedEntry10));
when(bridgeSupportMock.getLockWhitelistEntryByIndex(20)).then((InvocationOnMock invocation) -> Optional.of(mockedEntry20));

assertEquals(mockedEntry10.address().toBase58(), bridge.getLockWhitelistAddress(new Object[]{BigInteger.valueOf(10)}));
assertEquals(mockedEntry20.address().toBase58(), bridge.getLockWhitelistAddress(new Object[]{BigInteger.valueOf(20)}));
Expand Down Expand Up @@ -2227,9 +2227,9 @@ void getLockWhitelistEntryByAddressAfterRskip87Fork() throws Exception {

BridgeSupport bridgeSupportMock = mock(BridgeSupport.class);
when(bridgeSupportMock.getLockWhitelistEntryByAddress(mockedAddressForUnlimited.toBase58()))
.then((InvocationOnMock invocation) -> new UnlimitedWhiteListEntry(mockedAddressForUnlimited));
.then((InvocationOnMock invocation) -> Optional.of(new UnlimitedWhiteListEntry(mockedAddressForUnlimited)));
when(bridgeSupportMock.getLockWhitelistEntryByAddress(mockedAddressForOneOff.toBase58()))
.then((InvocationOnMock invocation) -> new OneOffWhiteListEntry(mockedAddressForOneOff, Coin.COIN));
.then((InvocationOnMock invocation) -> Optional.of(new OneOffWhiteListEntry(mockedAddressForOneOff, Coin.COIN)));

mockedTransaction = mock(Transaction.class);

Expand Down
65 changes: 40 additions & 25 deletions rskj-core/src/test/java/co/rsk/peg/whitelist/WhitelistSupportImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static co.rsk.peg.whitelist.WhitelistStorageIndexKey.LOCK_ONE_OFF;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import co.rsk.bitcoinj.core.Address;
import co.rsk.bitcoinj.core.Coin;
Expand All @@ -18,6 +17,8 @@
import java.math.BigInteger;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import org.apache.commons.lang3.tuple.Pair;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest;
Expand Down Expand Up @@ -65,44 +66,46 @@ void getLockWhitelistSize_whenLockWhitelistHasEntries_shouldReturnOne() {
}

@Test
void getLockWhitelistEntryByIndex_whenLockWhitelistIsEmpty_shouldReturnNull() {
LockWhitelistEntry actualEntry = whitelistSupport.getLockWhitelistEntryByIndex(0);
void getLockWhitelistEntryByIndex_whenLockWhitelistIsEmpty_shouldReturnAnEmptyOptional() {
Optional<LockWhitelistEntry> actualEntry = whitelistSupport.getLockWhitelistEntryByIndex(0);

assertNull(actualEntry);
assertTrue(actualEntry.isEmpty());
}

@Test
void getLockWhitelistEntryByIndex_whenLockWhitelistHasEntries_shouldReturnOneOffWhiteListEntry() {
saveInMemoryStorageOneOffWhiteListEntry();

LockWhitelistEntry actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByIndex(0);
Optional<LockWhitelistEntry> actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByIndex(0);

assertEquals(btcAddress, actualLockWhitelistEntry.address());
assertTrue(actualLockWhitelistEntry.isPresent());
assertEquals(btcAddress, actualLockWhitelistEntry.get().address());
}

@Test
void getLockWhitelistEntryByIndex_whenIndexIsOutOfBounds_shouldReturnNull() {
void getLockWhitelistEntryByIndex_whenIndexIsOutOfBounds_shouldReturnAnEmptyOptional() {
saveInMemoryStorageOneOffWhiteListEntry();

LockWhitelistEntry actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByIndex(1);
Optional<LockWhitelistEntry> actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByIndex(1);

assertNull(actualLockWhitelistEntry);
assertTrue(actualLockWhitelistEntry.isEmpty());
}

@Test
void getLockWhitelistEntryByAddress_whenLockWhitelistIsEmpty_shouldReturnNull() {
LockWhitelistEntry actualEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
void getLockWhitelistEntryByAddress_whenLockWhitelistIsEmpty_shouldReturnAnEmptyOptional() {
Optional<LockWhitelistEntry> actualEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());

assertNull(actualEntry);
assertTrue(actualEntry.isEmpty());
}

@Test
void getLockWhitelistEntryByAddress_whenLockWhitelistHasEntries_shouldReturnOneOffWhiteListEntry() {
saveInMemoryStorageOneOffWhiteListEntry();

LockWhitelistEntry actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
Optional<LockWhitelistEntry> actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());

assertEquals(btcAddress, actualLockWhitelistEntry.address());
assertTrue(actualLockWhitelistEntry.isPresent());
assertEquals(btcAddress, actualLockWhitelistEntry.get().address());
}

private void saveInMemoryStorageOneOffWhiteListEntry() {
Expand All @@ -120,9 +123,9 @@ private void saveInMemoryStorageOneOffWhiteListEntry() {
}

@Test
void getLockWhitelistEntryByAddress_whenAddressIsInvalid_shouldReturnNull() {
LockWhitelistEntry actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress("invalidAddress");
assertNull(actualLockWhitelistEntry);
void getLockWhitelistEntryByAddress_whenAddressIsInvalid_shouldReturnAnEmptyOptional() {
Optional<LockWhitelistEntry> actualLockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress("invalidAddress");
assertTrue(actualLockWhitelistEntry.isEmpty());
}

@Test
Expand Down Expand Up @@ -283,11 +286,12 @@ void setLockWhitelistDisableBlockDelay_whenDisableBlockDelayPlusBtcBlockchainBes
void verifyLockSenderIsWhitelisted_whenOneOffLockWhitelistAddressIsWhitelisted_shouldReturnTrue() {
Transaction tx = TransactionUtils.getTransactionFromCaller(signatureCache, WhitelistCaller.AUTHORIZED.getRskAddress());
whitelistSupport.addOneOffLockWhitelistAddress(tx, btcAddress.toString(), BigInteger.TEN);
LockWhitelistEntry lockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
Optional<LockWhitelistEntry> lockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());

boolean actualResult = whitelistSupport.verifyLockSenderIsWhitelisted(btcAddress, Coin.SATOSHI, 0);

assertTrue(lockWhitelistEntry.isConsumed());
assertTrue(lockWhitelistEntry.isPresent());
assertTrue(lockWhitelistEntry.get().isConsumed());
assertTrue(actualResult);
}

Expand All @@ -314,8 +318,8 @@ void save_whenLockWhitelistIsNull_shouldReturnZeroEntries() {

int actualSize = whitelistSupport.getLockWhitelistSize();
assertEquals(0, actualSize);
assertNull(whitelistSupport.getLockWhitelistEntryByIndex(0));
assertNull(whitelistSupport.getLockWhitelistEntryByIndex(1));
assertTrue(whitelistSupport.getLockWhitelistEntryByIndex(0).isEmpty());
assertTrue(whitelistSupport.getLockWhitelistEntryByIndex(1).isEmpty());
}

@Test
Expand All @@ -326,7 +330,10 @@ void save_whenOneOffLockWhitelistAddressIsWhitelisted_shouldSaveOneOffLockWhitel
whitelistSupport.save();

int actualSize = whitelistSupport.getLockWhitelistSize();
Address actualBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString()).address();
Optional<LockWhitelistEntry> lockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
assertTrue(lockWhitelistEntry.isPresent());

Address actualBtcAddress = lockWhitelistEntry.get().address();
assertEquals(1, actualSize);
assertEquals(btcAddress, actualBtcAddress);
}
Expand All @@ -339,7 +346,10 @@ void save_whenUnlimitedLockWhitelistAddressIsWhitelisted_shouldSaveUnlimitedLock
whitelistSupport.save();

int actualSize = whitelistSupport.getLockWhitelistSize();
Address actualBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString()).address();
Optional<LockWhitelistEntry> lockWhitelistEntry = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
assertTrue(lockWhitelistEntry.isPresent());

Address actualBtcAddress = lockWhitelistEntry.get().address();
assertEquals(1, actualSize);
assertEquals(btcAddress, actualBtcAddress);
}
Expand All @@ -353,8 +363,13 @@ void save_whenOneOffAndUnlimitedLockWhitelistAddressesAreWhitelisted_shouldSaveB
whitelistSupport.save();

int actualSize = whitelistSupport.getLockWhitelistSize();
Address actualBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString()).address();
Address actualSecondBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(secondBtcAddress.toString()).address();
Optional<LockWhitelistEntry> lockWhitelistEntryBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(btcAddress.toString());
assertTrue(lockWhitelistEntryBtcAddress.isPresent());
Address actualBtcAddress = lockWhitelistEntryBtcAddress.get().address();

Optional<LockWhitelistEntry> lockWhitelistEntrySecondBtcAddress = whitelistSupport.getLockWhitelistEntryByAddress(secondBtcAddress.toString());
assertTrue(lockWhitelistEntrySecondBtcAddress.isPresent());
Address actualSecondBtcAddress = lockWhitelistEntrySecondBtcAddress.get().address();
assertEquals(2, actualSize);
assertEquals(btcAddress, actualBtcAddress);
assertEquals(secondBtcAddress, actualSecondBtcAddress);
Expand Down

0 comments on commit 494c619

Please sign in to comment.