Skip to content

Commit 2fd1706

Browse files
authored
Merge pull request #390 from rsksmart/fix/remove-sigs-svp-spend
Remove signatures from svp spend tx before searching it while running HSM
2 parents b98c686 + 59e3a46 commit 2fd1706

File tree

3 files changed

+171
-7
lines changed

3 files changed

+171
-7
lines changed

src/main/java/co/rsk/federate/btcreleaseclient/BtcReleaseClient.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import co.rsk.peg.federation.ErpFederation;
4343
import co.rsk.peg.StateForFederator;
4444
import co.rsk.peg.StateForProposedFederator;
45+
import co.rsk.peg.bitcoin.BitcoinUtils;
4546
import java.time.Clock;
4647
import java.util.ArrayList;
4748
import java.util.Arrays;
@@ -66,9 +67,6 @@
6667
import org.ethereum.core.Block;
6768
import org.ethereum.core.TransactionReceipt;
6869
import org.ethereum.crypto.ECKey;
69-
import org.ethereum.db.BlockStore;
70-
import org.ethereum.db.ReceiptStore;
71-
import org.ethereum.db.TransactionInfo;
7270
import org.ethereum.facade.Ethereum;
7371
import org.ethereum.listener.EthereumListenerAdapter;
7472
import org.ethereum.util.RLP;
@@ -289,15 +287,22 @@ public void onBlock(org.ethereum.core.Block block, List<TransactionReceipt> rece
289287
* </p>
290288
*
291289
* @param currentBlockNumber the current block number in the blockchain
292-
* @param svpTxHash the Keccak256 hash of the svp spend transaction waiting to be signed
290+
* @param svpSpendTxEntry the Keccak256 hash and the Bitcoin transaction of the svp spend transaction waiting to be signed
293291
* @return {@code true} if the transaction has the required number of confirmations and is ready to be signed;
294292
* {@code false} otherwise
295293
*/
296-
private boolean isSVPSpendTxReadyToSign(long currentBlockNumber, Map.Entry<Keccak256, BtcTransaction> svpSpendTx) {
294+
private boolean isSVPSpendTxReadyToSign(long currentBlockNumber, Map.Entry<Keccak256, BtcTransaction> svpSpendTxEntry) {
297295
try {
296+
297+
BtcTransaction svpSpendTx = svpSpendTxEntry.getValue();
298+
299+
logger.debug("[isSvpSpendTxReadyToSign] SVP spend tx before removing signatures [{}]", svpSpendTx.getHash());
300+
BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(svpSpendTx);
301+
logger.debug("[isSvpSpendTxReadyToSign] SVP spend tx after removing signatures [{}]", svpSpendTx.getHash());
302+
298303
int version = signer.getVersionForKeyId(BTC.getKeyId());
299304
ReleaseCreationInformation releaseCreationInformation = releaseCreationInformationGetter.getTxInfoToSign(
300-
version, svpSpendTx.getKey(), svpSpendTx.getValue());
305+
version, svpSpendTxEntry.getKey(), svpSpendTx);
301306

302307
boolean isReadyToSign = Optional.ofNullable(releaseCreationInformation)
303308
.map(ReleaseCreationInformation::getPegoutCreationBlock)
@@ -307,7 +312,7 @@ private boolean isSVPSpendTxReadyToSign(long currentBlockNumber, Map.Entry<Kecca
307312
.isPresent();
308313

309314
logger.info("[isSvpSpendTxReadyToSign] SVP spend tx readiness check for signing: tx hash [{}], Current block [{}], Ready to sign? [{}]",
310-
svpSpendTx.getKey(),
315+
svpSpendTxEntry.getKey(),
311316
currentBlockNumber,
312317
isReadyToSign ? "YES" : "NO");
313318

src/test/java/co/rsk/federate/bitcoin/BitcoinTestUtils.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
package co.rsk.federate.bitcoin;
22

3+
import static co.rsk.bitcoinj.script.ScriptBuilder.createP2SHOutputScript;
4+
import static co.rsk.peg.bitcoin.BitcoinUtils.*;
5+
36
import co.rsk.bitcoinj.core.Address;
47
import co.rsk.bitcoinj.core.BtcECKey;
8+
import co.rsk.bitcoinj.core.BtcTransaction;
59
import co.rsk.bitcoinj.core.Coin;
610
import co.rsk.bitcoinj.core.NetworkParameters;
711
import co.rsk.bitcoinj.core.Sha256Hash;
12+
import co.rsk.bitcoinj.core.TransactionInput;
13+
import co.rsk.bitcoinj.crypto.TransactionSignature;
814
import co.rsk.bitcoinj.script.Script;
915
import co.rsk.bitcoinj.script.ScriptBuilder;
1016
import java.nio.charset.StandardCharsets;
@@ -66,4 +72,31 @@ public static Address createP2SHMultisigAddress(NetworkParameters networkParamet
6672
Script outputScript = ScriptBuilder.createP2SHOutputScript(redeemScript);
6773
return Address.fromP2SHScript(networkParameters, outputScript);
6874
}
75+
76+
public static void signTransactionInputFromP2shMultiSig(BtcTransaction transaction, int inputIndex, List<BtcECKey> keys) {
77+
if (transaction.getWitness(inputIndex).getPushCount() == 0) {
78+
signLegacyTransactionInputFromP2shMultiSig(transaction, inputIndex, keys);
79+
}
80+
}
81+
82+
private static void signLegacyTransactionInputFromP2shMultiSig(BtcTransaction transaction, int inputIndex, List<BtcECKey> keys) {
83+
TransactionInput input = transaction.getInput(inputIndex);
84+
85+
Script inputRedeemScript = extractRedeemScriptFromInput(input)
86+
.orElseThrow(() -> new IllegalArgumentException("Cannot sign inputs that are not from a p2sh multisig"));
87+
88+
Script outputScript = createP2SHOutputScript(inputRedeemScript);
89+
Sha256Hash sigHash = transaction.hashForSignature(inputIndex, inputRedeemScript, BtcTransaction.SigHash.ALL, false);
90+
Script inputScriptSig = input.getScriptSig();
91+
92+
for (BtcECKey key : keys) {
93+
BtcECKey.ECDSASignature sig = key.sign(sigHash);
94+
TransactionSignature txSig = new TransactionSignature(sig, BtcTransaction.SigHash.ALL, false);
95+
byte[] txSigEncoded = txSig.encodeToBitcoin();
96+
97+
int keyIndex = inputScriptSig.getSigInsertionIndex(sigHash, key);
98+
inputScriptSig = outputScript.getScriptSigWithSignature(inputScriptSig, txSigEncoded, keyIndex);
99+
input.setScriptSig(inputScriptSig);
100+
}
101+
}
69102
}

src/test/java/co/rsk/federate/btcreleaseclient/BtcReleaseClientTest.java

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.mockito.Mockito.verify;
2121
import static org.mockito.Mockito.verifyNoInteractions;
2222
import static org.mockito.Mockito.when;
23+
import static org.mockito.Mockito.spy;
2324

2425
import co.rsk.bitcoinj.core.BtcECKey;
2526
import co.rsk.bitcoinj.core.BtcTransaction;
@@ -32,6 +33,8 @@
3233
import co.rsk.bitcoinj.script.Script;
3334
import co.rsk.bitcoinj.script.ScriptBuilder;
3435
import co.rsk.bitcoinj.script.ScriptChunk;
36+
import co.rsk.federate.bitcoin.BitcoinTestUtils;
37+
import co.rsk.peg.bitcoin.BitcoinUtils;
3538
import co.rsk.peg.bitcoin.FlyoverRedeemScriptBuilderImpl;
3639
import co.rsk.peg.constants.BridgeConstants;
3740
import co.rsk.crypto.Keccak256;
@@ -765,6 +768,129 @@ void onBestBlock_whenOnlySvpSpendTxWaitingForSignaturesIsAvailable_shouldAddSign
765768
);
766769
}
767770

771+
@Test
772+
void onBestBlock_whenSvpSpendTxWaitingForSignaturesIsAvailableWithSignatureFromAnotherFederationMember_shouldSendAddSignature() throws Exception {
773+
// Arrange
774+
List<BtcECKey> federationKeys = TestUtils.getFederationPrivateKeys(9);
775+
Federation proposedFederation = TestUtils.createFederation(bridgeConstants.getBtcParams(), federationKeys);
776+
Script scriptSig = proposedFederation.getP2SHScript().createEmptyInputScript(null, proposedFederation.getRedeemScript());
777+
778+
BtcTransaction svpSpendTx = new BtcTransaction(params);
779+
svpSpendTx.addInput(BitcoinTestUtils.createHash(1), 0, scriptSig);
780+
svpSpendTx.addInput(BitcoinTestUtils.createHash(2), 0, scriptSig);
781+
svpSpendTx.addOutput(Coin.COIN, new BtcECKey().toAddress(params));
782+
Sha256Hash svpSpendTxHashBeforeSigning = svpSpendTx.getHash();
783+
784+
// Sign the svp spend tx
785+
List<TransactionInput> inputs = svpSpendTx.getInputs();
786+
for (TransactionInput input : inputs) {
787+
BitcoinTestUtils.signTransactionInputFromP2shMultiSig(
788+
svpSpendTx,
789+
inputs.indexOf(input),
790+
List.of(federationKeys.get(0))
791+
);
792+
}
793+
794+
Keccak256 svpSpendCreationRskTxHash = createHash(0);
795+
Map.Entry<Keccak256, BtcTransaction> svpSpendTxWFS = new AbstractMap.SimpleEntry<>(svpSpendCreationRskTxHash, svpSpendTx);
796+
StateForProposedFederator stateForProposedFederator = new StateForProposedFederator(svpSpendTxWFS);
797+
798+
Ethereum ethereum = mock(Ethereum.class);
799+
AtomicReference<EthereumListener> ethereumListener = new AtomicReference<>();
800+
doAnswer((InvocationOnMock invocation) -> {
801+
ethereumListener.set((EthereumListener) invocation.getArguments()[0]);
802+
return null;
803+
}).when(ethereum).addListener(any(EthereumListener.class));
804+
805+
FederatorSupport federatorSupport = mock(FederatorSupport.class);
806+
FederationMember federationMember = proposedFederation.getMembers().get(0);
807+
doReturn(federationMember).when(federatorSupport).getFederationMember();
808+
// return svp spend tx waiting for signatures
809+
doReturn(Optional.of(stateForProposedFederator)).when(federatorSupport).getStateForProposedFederator();
810+
// returns zero pegouts waiting for signatures
811+
doReturn(mock(StateForFederator.class)).when(federatorSupport).getStateForFederator();
812+
813+
ECKey ecKey = new ECKey();
814+
ECPublicKey signerPublicKey = new ECPublicKey(federationMember.getBtcPublicKey().getPubKey());
815+
816+
ECDSASigner signer = mock(ECDSASigner.class);
817+
doReturn(signerPublicKey).when(signer).getPublicKey(BTC.getKeyId());
818+
doReturn(1).when(signer).getVersionForKeyId(ArgumentMatchers.any(KeyId.class));
819+
doReturn(ecKey.doSign(new byte[]{})).when(signer).sign(any(KeyId.class), any(SignerMessage.class));
820+
821+
PowpegNodeSystemProperties powpegNodeSystemProperties = mock(PowpegNodeSystemProperties.class);
822+
doReturn(Constants.mainnet()).when(powpegNodeSystemProperties).getNetworkConstants();
823+
doReturn(true).when(powpegNodeSystemProperties).isPegoutEnabled();
824+
when(powpegNodeSystemProperties.getPegoutSignedCacheTtl())
825+
.thenReturn(PEGOUT_SIGNED_CACHE_TTL);
826+
827+
SignerMessageBuilderFactory signerMessageBuilderFactory = new SignerMessageBuilderFactory(
828+
mock(ReceiptStore.class)
829+
);
830+
831+
Keccak256 blockHash = createHash(2);
832+
Long blockNumber = 0L;
833+
Block block = mock(Block.class);
834+
TransactionReceipt txReceipt = mock(TransactionReceipt.class);
835+
TransactionInfo txInfo = mock(TransactionInfo.class);
836+
when(block.getHash()).thenReturn(blockHash);
837+
when(block.getNumber()).thenReturn(blockNumber);
838+
when(blockStore.getBlockByHash(blockHash.getBytes())).thenReturn(block);
839+
when(txInfo.getReceipt()).thenReturn(txReceipt);
840+
when(txInfo.getBlockHash()).thenReturn(blockHash.getBytes());
841+
when(receiptStore.getInMainChain(svpSpendCreationRskTxHash.getBytes(), blockStore)).thenReturn(Optional.of(txInfo));
842+
843+
ReleaseCreationInformationGetter releaseCreationInformationGetter =
844+
spy(new ReleaseCreationInformationGetter(
845+
receiptStore, blockStore
846+
));
847+
848+
BtcReleaseClientStorageSynchronizer storageSynchronizer =
849+
mock(BtcReleaseClientStorageSynchronizer.class);
850+
when(storageSynchronizer.isSynced()).thenReturn(true);
851+
852+
BtcReleaseClient btcReleaseClient = new BtcReleaseClient(
853+
ethereum,
854+
federatorSupport,
855+
powpegNodeSystemProperties,
856+
mock(NodeBlockProcessor.class)
857+
);
858+
859+
ActivationConfig activationConfig = mock(ActivationConfig.class);
860+
when(activationConfig.isActive(ConsensusRule.RSKIP419, bestBlock.getNumber())).thenReturn(true);
861+
862+
btcReleaseClient.setup(
863+
signer,
864+
activationConfig,
865+
signerMessageBuilderFactory,
866+
releaseCreationInformationGetter,
867+
mock(ReleaseRequirementsEnforcer.class),
868+
mock(BtcReleaseClientStorageAccessor.class),
869+
storageSynchronizer
870+
);
871+
872+
btcReleaseClient.start(proposedFederation);
873+
874+
// Act
875+
ethereumListener.get().onBestBlock(bestBlock, Collections.emptyList());
876+
877+
// Assert
878+
879+
// We should have searched by the expected svp spend tx hash
880+
BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(svpSpendTx);
881+
assertEquals(svpSpendTxHashBeforeSigning, svpSpendTx.getHash());
882+
verify(releaseCreationInformationGetter).getTxInfoToSign(
883+
anyInt(),
884+
eq(svpSpendCreationRskTxHash),
885+
eq(svpSpendTx));
886+
887+
// We should have added a signature for the svp spend tx
888+
verify(federatorSupport).addSignature(
889+
anyList(),
890+
any(byte[].class)
891+
);
892+
}
893+
768894
@Test
769895
void onBestBlock_whenBothPegoutAndSvpSpendTxWaitingForSignaturesAreAvailableAndFederatorIsOnlyPartOfProposedFederation_shouldOnlyAddOneSignature() throws Exception {
770896
// Arrange

0 commit comments

Comments
 (0)