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.

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

Refactor federation #2143

Merged
merged 27 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
53fc3e0
Refactor Federation classes. Remove all unnecessary overridden methods
julia-zack Sep 27, 2023
293030a
Add P2shP2wshErpFederation class for later
julia-zack Sep 27, 2023
b7aee41
Add some overrides needed. Fix some stuff
julia-zack Sep 27, 2023
1e04ebd
Modify getNumberOfSignaturesRequired method. Fix some tests
julia-zack Sep 28, 2023
de46d42
Fix getNumberOfSignaturesRequired
julia-zack Sep 28, 2023
daabd88
Refactor following the diagram
julia-zack Sep 29, 2023
efcb50b
Fix building non-tests-related errors
julia-zack Sep 29, 2023
d32cd4f
Fix tests
julia-zack Sep 30, 2023
f880a8d
Override Object methods in Federation. Make equals method different i…
julia-zack Sep 30, 2023
18bab72
Fix almost all tests.
julia-zack Oct 2, 2023
1b79ad9
Refactor some long lines
julia-zack Oct 2, 2023
4fdd864
Fix isPegOutTx. Rollback toString method from Federation
julia-zack Oct 3, 2023
fbb07af
Rename FederationTest. Refactor federation types
julia-zack Oct 3, 2023
4daf018
Refactor equals method
julia-zack Oct 3, 2023
fdab856
Add address to toString method from Federation
julia-zack Oct 3, 2023
a4c4399
Fix all tests
julia-zack Oct 3, 2023
21327ae
Remove unused and unnecessary code
julia-zack Oct 3, 2023
1e7826a
Modify equals method in Federation and remove some tests related. Ren…
julia-zack Oct 4, 2023
26435bb
Modify hashCode to be consistent with equals
julia-zack Oct 4, 2023
f6dce43
Fix sonar issues
julia-zack Oct 4, 2023
0ed881c
Rollback change
julia-zack Oct 4, 2023
d7fdf56
Fix and refactor tests.
julia-zack Oct 5, 2023
def7070
Comment unused-for-now code. Remove unnecessary string formatting
julia-zack Oct 5, 2023
a6ef2f7
Fix code smells
julia-zack Oct 6, 2023
0ad9ded
Add LEGACY_ to LEGACY_ERP variable
julia-zack Oct 6, 2023
05cabdf
Correct exception class. Remove TODOs. Remove comments
julia-zack Oct 6, 2023
c8a0ef3
Refactor test names. Remove unnecessary casting
julia-zack Oct 6, 2023
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
6 changes: 4 additions & 2 deletions rskj-core/src/main/java/co/rsk/config/BridgeConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
import co.rsk.peg.AddressBasedAuthorizer;
import co.rsk.peg.Federation;
import java.util.List;

import co.rsk.peg.StandardMultisigFederation;
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;

public abstract class BridgeConstants {
protected String btcParamsString;

protected Federation genesisFederation;
protected StandardMultisigFederation genesisFederation;
julia-zack marked this conversation as resolved.
Show resolved Hide resolved

protected int btc2RskMinimumAcceptableConfirmations;
protected int btc2RskMinimumAcceptableConfirmationsOnRsk;
Expand Down Expand Up @@ -94,7 +96,7 @@ public String getBtcParamsString() {
return btcParamsString;
}

public Federation getGenesisFederation() { return genesisFederation; }
public StandardMultisigFederation getGenesisFederation() { return genesisFederation; }

public int getBtc2RskMinimumAcceptableConfirmations() {
return btc2RskMinimumAcceptableConfirmations;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import co.rsk.peg.AddressBasedAuthorizer;
import co.rsk.peg.Federation;
import co.rsk.peg.FederationMember;
import co.rsk.peg.StandardMultisigFederation;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.crypto.ECKey;

Expand Down Expand Up @@ -58,7 +59,7 @@ public BridgeDevNetConstants(List<BtcECKey> federationPublicKeys) {

// Expected federation address is:
// 2NCEo1RdmGDj6MqiipD6DUSerSxKv79FNWX
genesisFederation = new Federation(
genesisFederation = new StandardMultisigFederation(
federationMembers,
genesisFederationAddressCreatedAt,
1L,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import co.rsk.peg.AddressBasedAuthorizer;
import co.rsk.peg.Federation;
import co.rsk.peg.FederationMember;
import co.rsk.peg.StandardMultisigFederation;
import com.google.common.collect.Lists;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.crypto.ECKey;
Expand Down Expand Up @@ -53,7 +54,7 @@ public class BridgeMainNetConstants extends BridgeConstants {
// Wednesday, January 3, 2018 12:00:00 AM GMT-03:00
Instant genesisFederationAddressCreatedAt = Instant.ofEpochMilli(1514948400l);

genesisFederation = new Federation(
genesisFederation = new StandardMultisigFederation(
federationMembers,
genesisFederationAddressCreatedAt,
1L,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import co.rsk.peg.AddressBasedAuthorizer;
import co.rsk.peg.Federation;
import co.rsk.peg.FederationMember;
import co.rsk.peg.StandardMultisigFederation;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.crypto.ECKey;
import org.ethereum.crypto.HashUtil;
Expand Down Expand Up @@ -56,7 +57,7 @@ public BridgeRegTestConstants(List<BtcECKey> federationPublicKeys) {

Instant genesisFederationCreatedAt = ZonedDateTime.parse("2016-01-01T00:00:00Z").toInstant();

genesisFederation = new Federation(
genesisFederation = new StandardMultisigFederation(
federationMembers,
genesisFederationCreatedAt,
1L,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import co.rsk.peg.AddressBasedAuthorizer;
import co.rsk.peg.Federation;
import co.rsk.peg.FederationMember;
import co.rsk.peg.StandardMultisigFederation;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.crypto.ECKey;

Expand Down Expand Up @@ -61,7 +62,7 @@ public class BridgeTestNetConstants extends BridgeConstants {
// Currently set to: Monday, October 8, 2018 12:00:00 AM GMT-03:00
Instant genesisFederationAddressCreatedAt = Instant.ofEpochMilli(1538967600l);

genesisFederation = new Federation(
genesisFederation = new StandardMultisigFederation(
federationMembers,
genesisFederationAddressCreatedAt,
1L,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ private static Federation deserializeFederationWithDeserializer(
federationMembers.add(member);
}

return new Federation(federationMembers, creationTime, creationBlockNumber, networkParameters);
return new StandardMultisigFederation(federationMembers, creationTime, creationBlockNumber, networkParameters);
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -326,7 +326,7 @@ public static ErpFederation deserializeErpFederation(
BridgeSerializationUtils::deserializeFederationMember
);

return new ErpFederation(
return new LegacyErpFederation(
marcos-iov marked this conversation as resolved.
Show resolved Hide resolved
federation.getMembers(),
federation.creationTime,
federation.getCreationBlockNumber(),
Expand Down
2 changes: 1 addition & 1 deletion rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -2079,7 +2079,7 @@ protected Integer commitFederation(boolean dryRun, Keccak256 hash) throws IOExce
// Preserve federation change info
long nextFederationCreationBlockHeight = rskExecutionBlock.getNumber();
provider.setNextFederationCreationBlockHeight(nextFederationCreationBlockHeight);
Script oldFederationP2SHScript = activations.isActive(RSKIP377)? oldFederation.getStandardP2SHScript(): oldFederation.getP2SHScript();
Script oldFederationP2SHScript = activations.isActive(RSKIP377)? ((ErpFederation) oldFederation).getStandardP2SHScript(): oldFederation.getP2SHScript();
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
provider.setLastRetiredFederationP2SHScript(oldFederationP2SHScript);
}

Expand Down
4 changes: 2 additions & 2 deletions rskj-core/src/main/java/co/rsk/peg/BridgeUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public static boolean isValidPegInTx(
throw new ScriptException(message);
}
Script inputStandardRedeemScript = redeemScriptParser.extractStandardRedeemScript();
if (activeFederations.stream().anyMatch(federation -> federation.getStandardRedeemScript().equals(inputStandardRedeemScript))) {
if (activeFederations.stream().anyMatch(federation -> (federation instanceof ErpFederation ? ((ErpFederation) federation).getStandardRedeemScript() : federation.getRedeemScript()).equals(inputStandardRedeemScript))) {
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

Expand Down Expand Up @@ -490,7 +490,7 @@ private static boolean isPegOutTx(BtcTransaction tx, Federation federation, Acti
}

public static boolean isPegOutTx(BtcTransaction tx, List<Federation> federations, ActivationConfig.ForBlock activations) {
return isPegOutTx(tx, activations, federations.stream().filter(Objects::nonNull).map(Federation::getStandardP2SHScript).toArray(Script[]::new));
return isPegOutTx(tx, activations, federations.stream().filter(Objects::nonNull).map(fed -> fed instanceof ErpFederation ? ((ErpFederation) fed).getStandardRedeemScript() : fed.getRedeemScript()).toArray(Script[]::new));
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
}

public static boolean isPegOutTx(BtcTransaction tx, ActivationConfig.ForBlock activations, Script... p2shScript) {
Expand Down
105 changes: 9 additions & 96 deletions rskj-core/src/main/java/co/rsk/peg/ErpFederation.java
Original file line number Diff line number Diff line change
@@ -1,30 +1,21 @@
package co.rsk.peg;

import co.rsk.bitcoinj.core.Address;
import co.rsk.bitcoinj.core.BtcECKey;
import co.rsk.bitcoinj.core.NetworkParameters;
import co.rsk.bitcoinj.script.ErpFederationRedeemScriptParser;
import co.rsk.bitcoinj.script.Script;
import co.rsk.bitcoinj.script.ScriptBuilder;
import co.rsk.bitcoinj.script.ScriptChunk;
import co.rsk.peg.utils.EcKeyUtils;
import java.time.Instant;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ErpFederation extends Federation {
private static final byte[] ERP_TESTNET_REDEEM_SCRIPT_BYTES = Hex.decode("6453210208f40073a9e43b3e9103acec79767a6de9b0409749884e989960fee578012fce210225e892391625854128c5c4ea4340de0c2a70570f33db53426fc9c746597a03f42102afc230c2d355b1a577682b07bc2646041b5d0177af0f98395a46018da699b6da210344a3c38cd59afcba3edcebe143e025574594b001700dec41e59409bdbd0f2a0921039a060badbeb24bee49eb2063f616c0f0f0765d4ca646b20a88ce828f259fcdb955670300cd50b27552210216c23b2ea8e4f11c3f9e22711addb1d16a93964796913830856b568cc3ea21d3210275562901dd8faae20de0a4166362a4f82188db77dbed4ca887422ea1ec185f1421034db69f2112f4fb1bb6141bf6e2bd6631f0484d0bd95b16767902c9fe219d4a6f5368ae");
private static final Logger logger = LoggerFactory.getLogger(ErpFederation.class);

public abstract class ErpFederation extends Federation {
Fixed Show fixed Hide fixed
protected final List<BtcECKey> erpPubKeys;
protected final long activationDelay;
protected final ActivationConfig.ForBlock activations;

protected Script standardRedeemScript;
protected Script standardP2SHScript;

Expand All @@ -45,7 +36,6 @@
// Try getting the redeem script in order to validate it can be built
// using the given public keys and csv value
getRedeemScript(); // NOSONAR
validateRedeemScript();
getStandardRedeemScript(); // NOSONAR
}

Expand All @@ -57,52 +47,8 @@
return activationDelay;
}

@Override
public Script getRedeemScript() {
if (!activations.isActive(ConsensusRule.RSKIP284) &&
btcParams.getId().equals(NetworkParameters.ID_TESTNET)) {
logger.debug("[getRedeemScript] Returning hardcoded redeem script");
return new Script(ERP_TESTNET_REDEEM_SCRIPT_BYTES);
}

if (redeemScript == null) {
logger.debug("[getRedeemScript] Creating the redeem script from the keys");
redeemScript = activations.isActive(ConsensusRule.RSKIP293) ?
ErpFederationRedeemScriptParser.createErpRedeemScript(
ScriptBuilder.createRedeemScript(getNumberOfSignaturesRequired(), getBtcPublicKeys()),
ScriptBuilder.createRedeemScript(erpPubKeys.size() / 2 + 1, erpPubKeys),
activationDelay
) :
ErpFederationRedeemScriptParser.createErpRedeemScriptDeprecated(
ScriptBuilder.createRedeemScript(getNumberOfSignaturesRequired(), getBtcPublicKeys()),
ScriptBuilder.createRedeemScript(erpPubKeys.size() / 2 + 1, erpPubKeys),
activationDelay
);
}

return redeemScript;
}

@Override
public Script getStandardRedeemScript() {
if (standardRedeemScript == null) {
standardRedeemScript = ErpFederationRedeemScriptParser.extractStandardRedeemScript(
getRedeemScript().getChunks()
);
}
return standardRedeemScript;
}

@Override
public Script getP2SHScript() {
if (p2shScript == null) {
p2shScript = ScriptBuilder.createP2SHOutputScript(getRedeemScript());
}
public abstract Script getStandardRedeemScript();

return p2shScript;
}

@Override
public Script getStandardP2SHScript() {
if (standardP2SHScript == null) {
standardP2SHScript = ScriptBuilder.createP2SHOutputScript(getStandardRedeemScript());
Expand All @@ -112,35 +58,13 @@
}

@Override
public Address getAddress() {
if (address == null) {
address = Address.fromP2SHScript(btcParams, getP2SHScript());
}
public int getNumberOfSignaturesRequired() {
List<ScriptChunk> standardRedeemScriptChunks = getStandardRedeemScript().getChunks();

return address;
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}

if (other == null || this.getClass() != other.getClass()) {
return false;
}

ErpFederation otherErpFederation = (ErpFederation) other;

return this.getNumberOfSignaturesRequired() == otherErpFederation.getNumberOfSignaturesRequired() &&
this.getSize() == otherErpFederation.getSize() &&
this.getCreationTime().equals(otherErpFederation.getCreationTime()) &&
this.creationBlockNumber == otherErpFederation.creationBlockNumber &&
this.btcParams.equals(otherErpFederation.btcParams) &&
this.members.equals(otherErpFederation.members) &&
this.getRedeemScript().equals(otherErpFederation.getRedeemScript()) &&
this.erpPubKeys.equals(otherErpFederation.erpPubKeys) &&
this.activationDelay == otherErpFederation.activationDelay;
// the threshold of a multisig is the first chunk of the redeemScript
// and the standardRedeemScript represents a multisig
ScriptChunk thresholdChunk = standardRedeemScriptChunks.get(0);
return Integer.parseInt(thresholdChunk.toString());
Fixed Show fixed Hide fixed
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
}

@Override
Expand All @@ -156,15 +80,4 @@
getActivationDelay()
);
}

private void validateRedeemScript() {
if (activations.isActive(ConsensusRule.RSKIP293) &&
this.redeemScript.equals(new Script(ERP_TESTNET_REDEEM_SCRIPT_BYTES))) {

String message = "Unable to create ERP Federation. The obtained redeem script matches the one hardcoded for testnet. "
+ "This would cause bitcoinj-thin to identify it as invalid";
logger.debug("[validateRedeemScript] {}", message);
throw new FederationCreationException(message);
}
}
}
Loading