-
Notifications
You must be signed in to change notification settings - Fork 267
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
Create a flyover redeem script builder #2727
Changes from 4 commits
8e0604a
0e71532
e90a050
2be66e4
7719ed1
08b28eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package co.rsk.peg.bitcoin; | ||
|
||
import co.rsk.bitcoinj.script.Script; | ||
import co.rsk.crypto.Keccak256; | ||
|
||
public interface FlyoverRedeemScriptBuilder { | ||
Script addFlyoverDerivationHashToRedeemScript(Keccak256 flyoverDerivationHash, Script redeemScript); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package co.rsk.peg.bitcoin; | ||
|
||
import co.rsk.bitcoinj.script.Script; | ||
import co.rsk.bitcoinj.script.ScriptBuilder; | ||
import co.rsk.bitcoinj.script.ScriptOpCodes; | ||
import co.rsk.crypto.Keccak256; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import static co.rsk.peg.bitcoin.RedeemScriptCreationException.Reason.INVALID_FLYOVER_DERIVATION_HASH; | ||
import static java.util.Objects.isNull; | ||
|
||
public class FlyoverRedeemScriptBuilderImpl implements FlyoverRedeemScriptBuilder { | ||
private static final Logger logger = LoggerFactory.getLogger(FlyoverRedeemScriptBuilderImpl.class); | ||
|
||
@Override | ||
public Script addFlyoverDerivationHashToRedeemScript(Keccak256 flyoverDerivationHash, Script redeemScript) { | ||
|
||
validateFlyoverDerivationHash(flyoverDerivationHash); | ||
|
||
ScriptBuilder scriptBuilder = new ScriptBuilder(); | ||
byte[] flyoverDerivationHashSerialized = flyoverDerivationHash.getBytes(); | ||
return scriptBuilder | ||
.data(flyoverDerivationHashSerialized) | ||
.op(ScriptOpCodes.OP_DROP) | ||
.addChunks(redeemScript.getChunks()) | ||
.build(); | ||
} | ||
|
||
private void validateFlyoverDerivationHash(Keccak256 flyoverDerivationHash) { | ||
if (isNull(flyoverDerivationHash) || flyoverDerivationHash.equals(Keccak256.ZERO_HASH)) { | ||
String message = String.format("Provided flyover derivation hash %s is invalid.", flyoverDerivationHash); | ||
logger.warn("[validateFlyoverDerivationHash] {}", message); | ||
throw new RedeemScriptCreationException(message, INVALID_FLYOVER_DERIVATION_HASH); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package co.rsk.peg.bitcoin; | ||
|
||
import co.rsk.bitcoinj.script.Script; | ||
import co.rsk.bitcoinj.script.ScriptChunk; | ||
import co.rsk.bitcoinj.script.ScriptOpCodes; | ||
import co.rsk.crypto.Keccak256; | ||
import co.rsk.peg.federation.Federation; | ||
import co.rsk.peg.federation.P2shErpFederationBuilder; | ||
import org.ethereum.TestUtils; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.MethodSource; | ||
|
||
import java.util.List; | ||
import java.util.stream.Stream; | ||
|
||
import static co.rsk.peg.bitcoin.RedeemScriptCreationException.Reason.INVALID_FLYOVER_DERIVATION_HASH; | ||
import static org.junit.jupiter.api.Assertions.*; | ||
|
||
class FlyoverRedeemScriptBuilderImplTest { | ||
private Script redeemScript; | ||
private FlyoverRedeemScriptBuilder flyoverRedeemScriptBuilder; | ||
|
||
@BeforeEach | ||
void setUp() { | ||
Federation federation = P2shErpFederationBuilder.builder().build(); | ||
redeemScript = federation.getRedeemScript(); | ||
flyoverRedeemScriptBuilder = new FlyoverRedeemScriptBuilderImpl(); | ||
} | ||
|
||
@ParameterizedTest | ||
@MethodSource("invalidDerivationHashArgsProvider") | ||
void addFlyoverDerivationHashToRedeemScript_withInvalidPrefix_shouldThrowFlyoverRedeemScriptCreationException(Keccak256 flyoverDerivationHash) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about improving test name adding
|
||
RedeemScriptCreationException exception = assertThrows(RedeemScriptCreationException.class, | ||
() -> flyoverRedeemScriptBuilder.addFlyoverDerivationHashToRedeemScript(flyoverDerivationHash, redeemScript)); | ||
|
||
assertEquals(INVALID_FLYOVER_DERIVATION_HASH, exception.getReason()); | ||
|
||
String expectedMessage = String.format("Provided flyover derivation hash %s is invalid.", flyoverDerivationHash); | ||
assertEquals(expectedMessage, exception.getMessage()); | ||
} | ||
|
||
private static Stream<Keccak256> invalidDerivationHashArgsProvider() { | ||
return Stream.of(null, Keccak256.ZERO_HASH); | ||
} | ||
|
||
@Test | ||
void addFlyoverDerivationHashToRedeemScript_shouldReturnRedeemScriptWithFlyoverDerivationHash() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// arrange | ||
Keccak256 flyoverDerivationHash = TestUtils.generateHash("hash"); | ||
|
||
// act | ||
Script redeemScriptWithFlyoverDerivationHash = flyoverRedeemScriptBuilder.addFlyoverDerivationHashToRedeemScript(flyoverDerivationHash, redeemScript); | ||
|
||
// assert | ||
List<ScriptChunk> originalRedeemScriptChunks = getOriginalRedeemScriptChunks(redeemScriptWithFlyoverDerivationHash); | ||
assertEquals(redeemScript.getChunks(), originalRedeemScriptChunks); | ||
|
||
List<ScriptChunk> redeemScriptWithFlyoverDerivationHashChunks = redeemScriptWithFlyoverDerivationHash.getChunks(); | ||
ScriptChunk flyoverDerivationHashChunk = redeemScriptWithFlyoverDerivationHashChunks.get(0); | ||
ScriptChunk opDropChunk = redeemScriptWithFlyoverDerivationHashChunks.get(1); | ||
assertArrayEquals(flyoverDerivationHash.getBytes(), flyoverDerivationHashChunk.data); | ||
assertEquals(ScriptOpCodes.OP_DROP, opDropChunk.opcode); | ||
} | ||
|
||
private List<ScriptChunk> getOriginalRedeemScriptChunks(Script redeemScript) { | ||
List<ScriptChunk> redeemScriptChunks = redeemScript.getChunks(); | ||
int firstOriginalChunkIndex = 2; | ||
int lastOriginalChunkIndex = redeemScriptChunks.size(); | ||
|
||
return redeemScriptChunks.subList(firstOriginalChunkIndex, lastOriginalChunkIndex); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why merge it into the
master
branch instead of the integration branch,wip/fed-scripts-refactors-integration
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes it easier to integrate all the changes we are working on in different branches