Skip to content
New issue

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

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

Already on GitHub? # to your account

Bridge Constants Refactor Integration #2309

Merged
merged 20 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
79cd63f
Refactoring BridgeConstants Class
wilmerrootstock Apr 1, 2024
82951e4
Refactoring BridgeConstants Class
wilmerrootstock Apr 2, 2024
049faf3
Addressing review comments
wilmerrootstock Apr 3, 2024
7cc926e
Refactoring BridgeConstants Class
wilmerrootstock Apr 1, 2024
bc0ba20
Remove dependency between BridgeConstants and Federation classes
wilmerrootstock Apr 17, 2024
551bd86
Addressing code review comments
wilmerrootstock Apr 18, 2024
a2d5315
Addressing review comments
wilmerrootstock Apr 18, 2024
b5cdaf3
Remove dependency between BridgeConstants and Federation classes
wilmerrootstock Apr 24, 2024
d040302
Remove dependency between BridgeConstants and Federation classes
wilmerrootstock Apr 25, 2024
0eeb559
Remove dependency between BridgeConstants and Federation classes
wilmerrootstock Apr 25, 2024
a5afc58
Fix failing tests by adapting test to new refactors on bridgeSupport …
nathanieliov Apr 25, 2024
9cf2974
Addressing Review comments:
wilmerrootstock Apr 25, 2024
24e6613
Addressing Variable Names Review Comments:
wilmerrootstock Apr 25, 2024
a4c68ab
Addressing review comments
wilmerrootstock Apr 26, 2024
d3d90a5
Review comments about replace ResgTest with MainNet
wilmerrootstock Apr 26, 2024
9a8f70c
Make Value field In KeyType Enum Final
wilmerrootstock Apr 26, 2024
dc57783
Fix log message in BridgeSupport releaseBtc Method
wilmerrootstock Apr 26, 2024
f878fd8
Fix log message in BridgeSupport releaseBtc Method
wilmerrootstock Apr 26, 2024
c70a501
Rename Minimum Pegout Constants From BridgeConstants
wilmerrootstock Apr 26, 2024
36ca811
Use original values for genesisFederationCreationTime
marcos-iov May 10, 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
2 changes: 1 addition & 1 deletion rskj-core/src/main/java/co/rsk/peg/Bridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import co.rsk.bitcoinj.core.*;
import co.rsk.bitcoinj.script.Script;
import co.rsk.bitcoinj.store.BlockStoreException;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.panic.PanicProcessor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import co.rsk.bitcoinj.core.*;
import co.rsk.bitcoinj.script.Script;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.peg.vote.ABICallElection;
Expand Down
2 changes: 1 addition & 1 deletion rskj-core/src/main/java/co/rsk/peg/BridgeState.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import co.rsk.bitcoinj.core.BtcTransaction;
import co.rsk.bitcoinj.core.UTXO;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.crypto.Keccak256;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import co.rsk.bitcoinj.core.*;
import co.rsk.bitcoinj.script.Script;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.peg.vote.ABICallElection;
Expand Down Expand Up @@ -127,7 +127,7 @@
this.bridgeConstants = bridgeConstants;
}

public List<UTXO> getNewFederationBtcUTXOs() throws IOException {

Check notice

Code scanning / CodeQL

Exposing internal representation Note

getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
getNewFederationBtcUTXOs exposes the internal representation stored in field newFederationBtcUTXOs. The value may be modified
after this call to getNewFederationBtcUTXOs
.
if (newFederationBtcUTXOs != null) {
return newFederationBtcUTXOs;
}
Expand Down
17 changes: 9 additions & 8 deletions rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import co.rsk.bitcoinj.store.BlockStoreException;
import co.rsk.bitcoinj.wallet.SendRequest;
import co.rsk.bitcoinj.wallet.Wallet;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.panic.PanicProcessor;
Expand Down Expand Up @@ -788,11 +788,12 @@ private void saveNewUTXOs(BtcTransaction btcTx) throws IOException {
* @throws IOException
*/
public void releaseBtc(Transaction rskTx) throws IOException {
Coin pegoutValue = rskTx.getValue().toBitcoin();
final co.rsk.core.Coin pegoutValueInWeis = rskTx.getValue();
final Coin pegoutValueInSatoshis = pegoutValueInWeis.toBitcoin();
final RskAddress senderAddress = rskTx.getSender(signatureCache);
logger.debug(
"[releaseBtc] Releasing {} RBTC from RSK address {} in tx {}",
pegoutValue,
"[releaseBtc] Releasing {} weis from RSK address {} in tx {}",
pegoutValueInWeis,
senderAddress,
rskTx.getHash()
);
Expand All @@ -804,7 +805,7 @@ public void releaseBtc(Transaction rskTx) throws IOException {
senderAddress
);
if (activations.isActive(ConsensusRule.RSKIP185)) {
emitRejectEvent(pegoutValue, senderAddress, RejectedPegoutReason.CALLER_CONTRACT);
emitRejectEvent(pegoutValueInSatoshis, senderAddress, RejectedPegoutReason.CALLER_CONTRACT);
return;
} else {
String message = "Contract calling releaseBTC";
Expand All @@ -818,7 +819,7 @@ public void releaseBtc(Transaction rskTx) throws IOException {
Address btcDestinationAddress = BridgeUtils.recoverBtcAddressFromEthTransaction(rskTx, btcParams);
logger.debug("[releaseBtc] BTC destination address: {}", btcDestinationAddress);

requestRelease(btcDestinationAddress, pegoutValue, rskTx);
requestRelease(btcDestinationAddress, pegoutValueInSatoshis, rskTx);
}

private void refundAndEmitRejectEvent(Coin value, RskAddress senderAddress, RejectedPegoutReason reason) {
Expand Down Expand Up @@ -868,7 +869,7 @@ private void requestRelease(Address destinationAddress, Coin value, Transaction
); // add the gap

// The pegout value should be greater or equals than the max of these two values
Coin minValue = Coin.valueOf(Math.max(bridgeConstants.getMinimumPegoutTxValueInSatoshis().value, requireFundsForFee.value));
Coin minValue = Coin.valueOf(Math.max(bridgeConstants.getMinimumPegoutTxValue().value, requireFundsForFee.value));

// Since Iris the peg-out the rule is that the minimum is inclusive
if (value.isLessThan(minValue)) {
Expand All @@ -880,7 +881,7 @@ private void requestRelease(Address destinationAddress, Coin value, Transaction
}
} else {
// For legacy peg-outs the rule stated that the minimum was exclusive
if (!value.isGreaterThan(bridgeConstants.getLegacyMinimumPegoutTxValueInSatoshis())) {
if (!value.isGreaterThan(bridgeConstants.getLegacyMinimumPegoutTxValue())) {
optionalRejectedPegoutReason = Optional.of(RejectedPegoutReason.LOW_AMOUNT);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package co.rsk.peg;

import co.rsk.bitcoinj.core.Context;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.peg.BtcBlockStoreWithCache.Factory;
import co.rsk.peg.btcLockSender.BtcLockSenderProvider;
Expand Down
48 changes: 34 additions & 14 deletions rskj-core/src/main/java/co/rsk/peg/BridgeUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import co.rsk.bitcoinj.script.Script;
import co.rsk.bitcoinj.script.ScriptChunk;
import co.rsk.bitcoinj.wallet.Wallet;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.peg.bitcoin.RskAllowUnconfirmedCoinSelector;
import co.rsk.peg.btcLockSender.BtcLockSender.TxSenderAddressType;
Expand All @@ -38,14 +38,17 @@
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.ethereum.core.SignatureCache;
import org.ethereum.core.Transaction;
import org.ethereum.crypto.ECKey;
import org.ethereum.util.ByteUtil;
import org.ethereum.vm.PrecompiledContracts;
import org.ethereum.vm.program.InternalTransaction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nonnull;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
Expand All @@ -56,11 +59,12 @@
/**
* @author Oscar Guindzberg
*/
public class BridgeUtils {
public final class BridgeUtils {

private static final Logger logger = LoggerFactory.getLogger(BridgeUtils.class);

private BridgeUtils() {}
private BridgeUtils() {
}

public static Wallet getFederationNoSpendWallet(
Context btcContext,
Expand Down Expand Up @@ -153,7 +157,7 @@ public static Coin getAmountSentToAddresses(
if (addresses == null || addresses.isEmpty()){
return Coin.ZERO;
}
if (activations.isActive(ConsensusRule.RSKIP293)){
if (activations.isActive(RSKIP293)){
return getAmountSentToAddresses(
context,
btcTx,
Expand Down Expand Up @@ -252,7 +256,7 @@ public static List<UTXO> getUTXOsSentToAddresses(
List<Address> addresses

) {
if (activations.isActive(ConsensusRule.RSKIP293)){
if (activations.isActive(RSKIP293)){
return getUTXOsSentToAddresses(context, btcTx, addresses);
} else {
return BridgeUtilsLegacy.getUTXOsSentToAddress(
Expand Down Expand Up @@ -379,8 +383,8 @@ public static boolean hasEnoughSignatures(Context btcContext, BtcTransaction btc
return true;
}

public static Address recoverBtcAddressFromEthTransaction(org.ethereum.core.Transaction tx, NetworkParameters networkParameters) {
org.ethereum.crypto.ECKey key = tx.getKey();
public static Address recoverBtcAddressFromEthTransaction(Transaction tx, NetworkParameters networkParameters) {
ECKey key = tx.getKey();
byte[] pubKey = key.getPubKey(true);
return BtcECKey.fromPublicOnly(pubKey).toAddress(networkParameters);
}
Expand All @@ -392,6 +396,7 @@ public static boolean isFreeBridgeTx(Transaction rskTx, Constants constants, Act
}

BridgeConstants bridgeConstants = constants.getBridgeConstants();
RskAddress senderAddress = rskTx.getSender(signatureCache);

// Temporary assumption: if areBridgeTxsFree() is true then the current federation
// must be the genesis federation.
Expand All @@ -400,7 +405,7 @@ public static boolean isFreeBridgeTx(Transaction rskTx, Constants constants, Act
!activations.isActive(ConsensusRule.ARE_BRIDGE_TXS_PAID) &&
rskTx.acceptTransactionSignature(constants.getChainId()) &&
(
isFromFederateMember(rskTx, bridgeConstants.getGenesisFederation(), signatureCache) ||
isFromGenesisFederation(senderAddress, bridgeConstants.getGenesisFederationPublicKeys()) ||
isFromFederationChangeAuthorizedSender(rskTx, bridgeConstants, signatureCache) ||
isFromLockWhitelistChangeAuthorizedSender(rskTx, bridgeConstants, signatureCache) ||
isFromFeePerKbChangeAuthorizedSender(rskTx, bridgeConstants, signatureCache)
Expand All @@ -414,13 +419,28 @@ public static boolean isFreeBridgeTx(Transaction rskTx, Constants constants, Act
*/
public static boolean isContractTx(Transaction rskTx) {
// Calls between contracts are done through internal transactions
return rskTx.getClass() == org.ethereum.vm.program.InternalTransaction.class;
return rskTx.getClass() == InternalTransaction.class;
}

public static boolean isFromFederateMember(org.ethereum.core.Transaction rskTx, Federation federation, SignatureCache signatureCache) {
public static boolean isFromFederateMember(Transaction rskTx, Federation federation, SignatureCache signatureCache) {
return federation.hasMemberWithRskAddress(rskTx.getSender(signatureCache).getBytes());
}

/**
* Method that verify if an RskAddress is part of the Genesis Federation
*
* @param rskAddress RskAddress to find in Genesis Federation
* @param genesisFederationPublicKeys List of BtcECKey part of Genesis Federation
* @return boolean
*/
private static boolean isFromGenesisFederation(RskAddress rskAddress, List<BtcECKey> genesisFederationPublicKeys) {
return genesisFederationPublicKeys.stream().anyMatch(genesisBtcPublicKey -> {
ECKey genesisEcKey = ECKey.fromPublicOnly(genesisBtcPublicKey.getPubKey());
return Arrays.equals(genesisEcKey.getAddress(), rskAddress.getBytes());
}
);
}

public static Coin getCoinFromBigInteger(BigInteger value) throws BridgeIllegalArgumentException {
if (value == null) {
throw new BridgeIllegalArgumentException("value cannot be null");
Expand All @@ -432,17 +452,17 @@ public static Coin getCoinFromBigInteger(BigInteger value) throws BridgeIllegalA
}
}

private static boolean isFromFederationChangeAuthorizedSender(org.ethereum.core.Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
private static boolean isFromFederationChangeAuthorizedSender(Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
AddressBasedAuthorizer authorizer = bridgeConfiguration.getFederationChangeAuthorizer();
return authorizer.isAuthorized(rskTx, signatureCache);
}

private static boolean isFromLockWhitelistChangeAuthorizedSender(org.ethereum.core.Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
private static boolean isFromLockWhitelistChangeAuthorizedSender(Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
AddressBasedAuthorizer authorizer = bridgeConfiguration.getLockWhitelistChangeAuthorizer();
return authorizer.isAuthorized(rskTx, signatureCache);
}

private static boolean isFromFeePerKbChangeAuthorizedSender(org.ethereum.core.Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
private static boolean isFromFeePerKbChangeAuthorizedSender(Transaction rskTx, BridgeConstants bridgeConfiguration, SignatureCache signatureCache) {
AddressBasedAuthorizer authorizer = bridgeConfiguration.getFeePerKbChangeAuthorizer();
return authorizer.isAuthorized(rskTx, signatureCache);
}
Expand Down Expand Up @@ -541,7 +561,7 @@ public static Address deserializeBtcAddressWithVersion(
ActivationConfig.ForBlock activations,
byte[] addressBytes) throws BridgeIllegalArgumentException {

if (!activations.isActive(ConsensusRule.RSKIP284)) {
if (!activations.isActive(RSKIP284)) {
return BridgeUtilsLegacy.deserializeBtcAddressWithVersionLegacy(networkParameters, activations, addressBytes);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import co.rsk.bitcoinj.core.StoredBlock;
import co.rsk.bitcoinj.store.BlockStoreException;
import co.rsk.bitcoinj.store.BtcBlockStore;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import org.ethereum.config.blockchain.upgrades.ActivationConfig.ForBlock;
import org.ethereum.core.Repository;

Expand Down
21 changes: 19 additions & 2 deletions rskj-core/src/main/java/co/rsk/peg/FederationSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@

import co.rsk.bitcoinj.core.BtcECKey;
import co.rsk.bitcoinj.core.UTXO;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.peg.federation.Federation;
import co.rsk.peg.federation.FederationArgs;
import co.rsk.peg.federation.FederationFactory;
import co.rsk.peg.federation.FederationMember;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.core.Block;

import javax.annotation.Nullable;
import java.io.IOException;
import java.time.Instant;
import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -110,7 +113,7 @@ public Federation getActiveFederation() {
return provider.getOldFederation();
case GENESIS:
default:
return bridgeConstants.getGenesisFederation();
return this.getGenesisFederation();
}
}

Expand Down Expand Up @@ -233,4 +236,18 @@ private boolean shouldFederationBeActive(Federation federation) {
long federationAge = executionBlock.getNumber() - federation.getCreationBlockNumber();
return federationAge >= bridgeConstants.getFederationActivationAge(activations);
}

/**
* Get the Genesis Federation from a List of Genesis Federation Public Keys
*
* @return Federation
*/
private Federation getGenesisFederation() {
final long GENESIS_FEDERATION_CREATION_BLOCK_NUMBER = 1L;
final List<BtcECKey> genesisFederationPublicKeys = bridgeConstants.getGenesisFederationPublicKeys();
final List<FederationMember> federationMembers = FederationMember.getFederationMembersFromKeys(genesisFederationPublicKeys);
final Instant genesisFederationCreationTime = bridgeConstants.getGenesisFederationCreationTime();
final FederationArgs federationArgs = new FederationArgs(federationMembers, genesisFederationCreationTime, GENESIS_FEDERATION_CREATION_BLOCK_NUMBER, bridgeConstants.getBtcParams());
return FederationFactory.buildStandardMultiSigFederation(federationArgs);
}
}
2 changes: 1 addition & 1 deletion rskj-core/src/main/java/co/rsk/peg/PegUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import co.rsk.bitcoinj.core.TransactionOutput;
import co.rsk.bitcoinj.script.Script;
import co.rsk.bitcoinj.wallet.Wallet;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.peg.bitcoin.BitcoinUtils;
import co.rsk.peg.btcLockSender.BtcLockSender.TxSenderAddressType;
import co.rsk.peg.federation.Federation;
Expand Down
2 changes: 1 addition & 1 deletion rskj-core/src/main/java/co/rsk/peg/PegUtilsLegacy.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import co.rsk.bitcoinj.script.ScriptChunk;
import co.rsk.bitcoinj.script.ScriptOpCodes;
import co.rsk.bitcoinj.wallet.Wallet;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.peg.bitcoin.BitcoinUtils;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import co.rsk.bitcoinj.core.Sha256Hash;
import co.rsk.bitcoinj.core.StoredBlock;
import co.rsk.bitcoinj.store.BlockStoreException;
import co.rsk.config.BridgeConstants;
import co.rsk.peg.constants.BridgeConstants;
import co.rsk.core.RskAddress;
import co.rsk.util.MaxSizeHashMap;
import java.util.Optional;
Expand Down
Loading
Loading