Skip to content

Commit

Permalink
Merge pull request #2262 from rsksmart/handle-null-response
Browse files Browse the repository at this point in the history 
Handle null response
  • Loading branch information
@Vovchyk
Vovchyk authored Mar 5, 2024
2 parents 6cb7531 + fc4e43a commit 15f1669
Show file tree
Hide file tree
Showing 17 changed files with 3,471 additions and 1,009 deletions.
173 changes: 128 additions & 45 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 @@ -17,13 +17,16 @@
*/
package co.rsk.peg;

import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP417;

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.core.RskAddress;
import co.rsk.crypto.Keccak256;
import co.rsk.panic.PanicProcessor;
import co.rsk.peg.BridgeMethods.BridgeMethodExecutor;
import co.rsk.peg.bitcoin.MerkleBranch;
import co.rsk.peg.flyover.FlyoverTxResponseCodes;
import co.rsk.peg.utils.BtcTransactionFormatUtils;
Expand All @@ -37,6 +40,7 @@
import org.ethereum.core.*;
import org.ethereum.util.ByteUtil;
import org.ethereum.vm.DataWord;
import org.ethereum.vm.MessageCall.MsgType;
import org.ethereum.vm.PrecompiledContractArgs;
import org.ethereum.vm.PrecompiledContracts;
import org.ethereum.vm.exception.VMException;
Expand Down Expand Up @@ -223,15 +227,34 @@ public class Bridge extends PrecompiledContracts.PrecompiledContract {
private final BiFunction<List<Sha256Hash>, Integer, MerkleBranch> merkleBranchFactory;

private final SignatureCache signatureCache;

public Bridge(RskAddress contractAddress, Constants constants, ActivationConfig activationConfig,
BridgeSupportFactory bridgeSupportFactory, SignatureCache signatureCache) {
this(contractAddress, constants, activationConfig, bridgeSupportFactory, MerkleBranch::new, signatureCache);
private MsgType msgType;

public Bridge(
RskAddress contractAddress,
Constants constants,
ActivationConfig activationConfig,
BridgeSupportFactory bridgeSupportFactory,
SignatureCache signatureCache) {

this(
contractAddress,
constants,
activationConfig,
bridgeSupportFactory,
MerkleBranch::new,
signatureCache
);
}

@VisibleForTesting
Bridge(RskAddress contractAddress, Constants constants, ActivationConfig activationConfig,
BridgeSupportFactory bridgeSupportFactory, BiFunction<List<Sha256Hash>, Integer, MerkleBranch> merkleBranchFactory, SignatureCache signatureCache) {
Bridge(
RskAddress contractAddress,
Constants constants,
ActivationConfig activationConfig,
BridgeSupportFactory bridgeSupportFactory,
BiFunction<List<Sha256Hash>, Integer, MerkleBranch> merkleBranchFactory,
SignatureCache signatureCache) {

this.bridgeSupportFactory = bridgeSupportFactory;
this.contractAddress = contractAddress;
this.constants = constants;
Expand Down Expand Up @@ -316,12 +339,14 @@ public void init(PrecompiledContractArgs args) {
Block rskExecutionBlock = args.getExecutionBlock();
this.activations = activationConfig.forBlock(rskExecutionBlock.getNumber());
this.rskTx = args.getTransaction();
this.msgType = args.getMsgType();

this.bridgeSupport = bridgeSupportFactory.newInstance(
args.getRepository(),
rskExecutionBlock,
contractAddress,
args.getLogs());
args.getRepository(),
rskExecutionBlock,
contractAddress,
args.getLogs()
);
}

@Override
Expand All @@ -342,50 +367,98 @@ public byte[] execute(byte[] data) throws VMException {
// Function parsing from data returned null => invalid function selected, halt!
if (bridgeParsedData == null) {
String errorMessage = String.format("Invalid data given: %s.", ByteUtil.toHexString(data));
logger.info(errorMessage);
if (activations.isActive(ConsensusRule.RSKIP88)) {
throw new BridgeIllegalArgumentException(errorMessage);
logger.info("[execute] {}", errorMessage);
if (!activations.isActive(ConsensusRule.RSKIP88)) {
return null;
}

return null;
}

// If this is not a local call, then first check whether the function
// allows for non-local calls
if (activations.isActive(ConsensusRule.RSKIP88) &&
!isLocalCall() &&
bridgeParsedData.bridgeMethod.onlyAllowsLocalCalls(this, bridgeParsedData.args)) {

String errorMessage = String.format("Non-local-call to %s. Returning without execution.", bridgeParsedData.bridgeMethod.getFunction().name);
logger.info(errorMessage);
throw new BridgeIllegalArgumentException(errorMessage);
}

validateCall(bridgeParsedData);
Optional<?> result;
try {
// bridgeParsedData.function should be one of the CallTransaction.Function declared above.
// If the user tries to call an non-existent function, parseData() will return null.
result = bridgeParsedData.bridgeMethod.getExecutor().execute(this, bridgeParsedData.args);
result = executeBridgeMethod(bridgeParsedData);
} catch (BridgeIllegalArgumentException ex) {
String errorMessage = String.format("Error executing: %s", bridgeParsedData.bridgeMethod);
logger.warn(errorMessage, ex);
if (activations.isActive(ConsensusRule.RSKIP88)) {
throw new BridgeIllegalArgumentException(errorMessage);
if (shouldReturnNullInsteadOfException()) {
return null;
}

return null;
throw ex;
}

teardown();

return result.map(bridgeParsedData.bridgeMethod.getFunction()::encodeOutputs).orElse(null);
byte[] voidReturnValue = calculateVoidReturnValue();
return result.map(bridgeParsedData.bridgeMethod.getFunction()::encodeOutputs).orElse(voidReturnValue);
} catch (Exception ex) {
logger.error(ex.getMessage(), ex);
panicProcessor.panic("bridgeexecute", ex.getMessage());
throw new VMException(String.format("Exception executing bridge: %s", ex.getMessage()), ex);
}
}

private void validateCall(BridgeParsedData bridgeParsedData) throws BridgeIllegalArgumentException {
validateLocalCall(bridgeParsedData);
validateCallMessageType(bridgeParsedData);
}

private void validateLocalCall(BridgeParsedData bridgeParsedData) throws BridgeIllegalArgumentException {
// If this is not a local call, then check whether the function allows for non-local calls
if (activations.isActive(ConsensusRule.RSKIP88) &&
!isLocalCall() &&
bridgeParsedData.bridgeMethod.onlyAllowsLocalCalls(this, bridgeParsedData.args)) {

String errorMessage = String.format(
"Non-local-call to %s. Returning without execution.",
bridgeParsedData.bridgeMethod.getFunction().name
);
logger.info("[validateLocalCall] {}", errorMessage);
throw new BridgeIllegalArgumentException(errorMessage);
}
}

private void validateCallMessageType(BridgeParsedData bridgeParsedData) throws BridgeIllegalArgumentException {
if (activations.isActive(RSKIP417) &&
!bridgeParsedData.bridgeMethod.acceptsThisTypeOfCall(this.msgType)) {
String errorMessage = String.format(
"Call type (%s) not accepted by %s. Returning without execution.",
this.msgType.name(),
bridgeParsedData.bridgeMethod.getFunction().name
);
logger.info("[validateCallMessageType] {}", errorMessage);

throw new BridgeIllegalArgumentException(errorMessage);
}
}

private Optional<?> executeBridgeMethod(BridgeParsedData bridgeParsedData) throws Exception {
try {
// bridgeParsedData.function should be one of the CallTransaction.Function declared above.
// If the user tries to call an non-existent function, parseData() will return null.
BridgeMethodExecutor executor = bridgeParsedData.bridgeMethod.getExecutor();
return executor.execute(this, bridgeParsedData.args);
} catch (BridgeIllegalArgumentException ex) {
String errorMessage = String.format("Error executing: %s", bridgeParsedData.bridgeMethod);
logger.warn(errorMessage, ex);

throw new BridgeIllegalArgumentException(errorMessage);
}
}

private boolean shouldReturnNullInsteadOfException() {
return !activations.isActive(ConsensusRule.RSKIP88);
}

private byte[] calculateVoidReturnValue() {
if (shouldReturnNullOnVoidMethods()) {
return null;
}
return new byte[]{};
}

private boolean shouldReturnNullOnVoidMethods() {
return !activations.isActive(RSKIP417);
}

private void teardown() throws IOException {
bridgeSupport.save();
}
Expand Down Expand Up @@ -628,7 +701,7 @@ public byte[] getBtcBlockchainBlockHashAtDepth(Object[] args) throws VMException
logger.trace("getBtcBlockchainBlockHashAtDepth");

int depth = ((BigInteger) args[0]).intValue();
Sha256Hash blockHash = null;
Sha256Hash blockHash;
try {
blockHash = bridgeSupport.getBtcBlockchainBlockHashAtDepth(depth);
} catch (Exception e) {
Expand Down Expand Up @@ -831,8 +904,8 @@ public Integer createFederation(Object[] args) throws BridgeIllegalArgumentExcep
logger.trace("createFederation");

return bridgeSupport.voteFederationChange(
rskTx,
new ABICallSpec("create", new byte[][]{})
rskTx,
new ABICallSpec("create", new byte[][]{})
);
}

Expand All @@ -848,8 +921,8 @@ public Integer addFederatorPublicKey(Object[] args) throws BridgeIllegalArgument
}

return bridgeSupport.voteFederationChange(
rskTx,
new ABICallSpec("add", new byte[][]{ publicKeyBytes })
rskTx,
new ABICallSpec("add", new byte[][]{ publicKeyBytes })
);
}

Expand All @@ -861,8 +934,11 @@ public Integer addFederatorPublicKeyMultikey(Object[] args) throws BridgeIllegal
byte[] mstPublicKeyBytes = (byte[]) args[2];

return bridgeSupport.voteFederationChange(
rskTx,
new ABICallSpec("add-multi", new byte[][]{ btcPublicKeyBytes, rskPublicKeyBytes, mstPublicKeyBytes })
rskTx,
new ABICallSpec(
"add-multi",
new byte[][]{ btcPublicKeyBytes, rskPublicKeyBytes, mstPublicKeyBytes }
)
);
}

Expand All @@ -878,8 +954,8 @@ public Integer commitFederation(Object[] args) throws BridgeIllegalArgumentExcep
}

return bridgeSupport.voteFederationChange(
rskTx,
new ABICallSpec("commit", new byte[][]{ hash })
rskTx,
new ABICallSpec("commit", new byte[][]{ hash })
);
}

Expand Down Expand Up @@ -1100,7 +1176,14 @@ public void registerBtcCoinbaseTransaction(Object[] args) throws VMException {
byte[] pmtSerialized = (byte[]) args[2];
Sha256Hash witnessMerkleRoot = Sha256Hash.wrap((byte[]) args[3]);
byte[] witnessReservedValue = (byte[]) args[4];
bridgeSupport.registerBtcCoinbaseTransaction(btcTxSerialized, blockHash, pmtSerialized, witnessMerkleRoot, witnessReservedValue);

bridgeSupport.registerBtcCoinbaseTransaction(
btcTxSerialized,
blockHash,
pmtSerialized,
witnessMerkleRoot,
witnessReservedValue
);
}

public boolean hasBtcBlockCoinbaseTransactionInformation(Object[] args) {
Expand Down
Loading

0 comments on commit 15f1669

Please sign in to comment.