diff --git a/solidity/contracts/bridge/BitcoinTx.sol b/solidity/contracts/bridge/BitcoinTx.sol index 8091be4ca..d78a9af49 100644 --- a/solidity/contracts/bridge/BitcoinTx.sol +++ b/solidity/contracts/bridge/BitcoinTx.sol @@ -278,49 +278,82 @@ library BitcoinTx { pubKeyHash = pubKeyHashBytes.slice20(0); - // We need to make sure that the 20-byte public key hash - // is actually used in the right context of a P2PKH or P2WPKH - // output. To do so, we must extract the full script from the output - // and compare with the expected P2PKH and P2WPKH scripts - // referring to that 20-byte public key hash. The output consists - // of an 8-byte value and a variable length script. To extract the - // script we slice the output starting from 9th byte until the end. - bytes32 outputScriptKeccak = keccak256( - output.slice(8, output.length - 8) - ); - // Build the expected P2PKH script which has the following byte - // format: <0x1976a914> <20-byte PKH> <0x88ac>. According to - // https://en.bitcoin.it/wiki/Script#Opcodes this translates to: - // - 0x19: Byte length of the entire script - // - 0x76: OP_DUP - // - 0xa9: OP_HASH160 - // - 0x14: Byte length of the public key hash - // - 0x88: OP_EQUALVERIFY - // - 0xac: OP_CHECKSIG - // which matches the P2PKH structure as per: - // https://en.bitcoin.it/wiki/Transaction#Pay-to-PubkeyHash - bytes32 P2PKHScriptKeccak = keccak256( - abi.encodePacked(hex"1976a914", pubKeyHash, hex"88ac") - ); - // Build the expected P2WPKH script which has the following format: - // <0x160014> <20-byte PKH>. According to - // https://en.bitcoin.it/wiki/Script#Opcodes this translates to: - // - 0x16: Byte length of the entire script - // - 0x00: OP_0 - // - 0x14: Byte length of the public key hash - // which matches the P2WPKH structure as per: - // https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#P2WPKH - bytes32 P2WPKHScriptKeccak = keccak256( - abi.encodePacked(hex"160014", pubKeyHash) - ); - // Make sure the actual output script matches either the P2PKH - // or P2WPKH format. + uint256 scriptLen = output.length - 8; + + // The P2PKH script is 26 bytes long. + // The P2WPKH script is 23 bytes long. + // A valid script must have one of these lengths, + // and we can identify the expected script type by the length. require( - outputScriptKeccak == P2PKHScriptKeccak || - outputScriptKeccak == P2WPKHScriptKeccak, + scriptLen == 26 || scriptLen == 23, "Output must be P2PKH or P2WPKH" ); + if (scriptLen == 26) { + // Compare to the expected P2PKH script. + bytes26 script = bytes26(output.slice32(8)); + + require( + script == makeP2PKHScript(pubKeyHash), + "Invalid P2PKH script" + ); + } + + if (scriptLen == 23) { + // Compare to the expected P2WPKH script. + bytes23 script = bytes23(output.slice32(8)); + + require( + script == makeP2WPKHScript(pubKeyHash), + "Invalid P2WPKH script" + ); + } + return pubKeyHash; } + + /// @notice Build the P2PKH script from the given public key hash. + /// @param pubKeyHash The 20-byte public key hash. + /// @return The P2PKH script. + /// @dev The P2PKH script has the following byte format: + /// <0x1976a914> <20-byte PKH> <0x88ac>. According to + /// https://en.bitcoin.it/wiki/Script#Opcodes this translates to: + /// - 0x19: Byte length of the entire script + /// - 0x76: OP_DUP + /// - 0xa9: OP_HASH160 + /// - 0x14: Byte length of the public key hash + /// - 0x88: OP_EQUALVERIFY + /// - 0xac: OP_CHECKSIG + /// which matches the P2PKH structure as per: + /// https://en.bitcoin.it/wiki/Transaction#Pay-to-PubkeyHash + function makeP2PKHScript(bytes20 pubKeyHash) + internal + pure + returns (bytes26) + { + bytes26 P2PKHScriptMask = hex"1976a914000000000000000000000000000000000000000088ac"; + + return ((bytes26(pubKeyHash) >> 32) | P2PKHScriptMask); + } + + /// @notice Build the P2WPKH script from the given public key hash. + /// @param pubKeyHash The 20-byte public key hash. + /// @return The P2WPKH script. + /// @dev The P2WPKH script has the following format: + /// <0x160014> <20-byte PKH>. According to + /// https://en.bitcoin.it/wiki/Script#Opcodes this translates to: + /// - 0x16: Byte length of the entire script + /// - 0x00: OP_0 + /// - 0x14: Byte length of the public key hash + /// which matches the P2WPKH structure as per: + /// https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#P2WPKH + function makeP2WPKHScript(bytes20 pubKeyHash) + internal + pure + returns (bytes23) + { + bytes23 P2WPKHScriptMask = hex"1600140000000000000000000000000000000000000000"; + + return ((bytes23(pubKeyHash) >> 24) | P2WPKHScriptMask); + } } diff --git a/solidity/contracts/bridge/Bridge.sol b/solidity/contracts/bridge/Bridge.sol index dc310620c..c7d148d4e 100644 --- a/solidity/contracts/bridge/Bridge.sol +++ b/solidity/contracts/bridge/Bridge.sol @@ -239,7 +239,7 @@ contract Bridge is address _relay, address _treasury, address _ecdsaWalletRegistry, - uint256 _txProofDifficultyFactor + uint96 _txProofDifficultyFactor ) external initializer { require(_bank != address(0), "Bank address cannot be zero"); self.bank = Bank(_bank); @@ -1228,9 +1228,9 @@ contract Bridge is uint64 redemptionDustThreshold, uint64 redemptionTreasuryFeeDivisor, uint64 redemptionTxMaxFee, - uint256 redemptionTimeout, + uint64 redemptionTimeout, uint96 redemptionTimeoutSlashingAmount, - uint256 redemptionTimeoutNotifierRewardMultiplier + uint64 redemptionTimeoutNotifierRewardMultiplier ) external onlyGovernance { self.updateRedemptionParameters( redemptionDustThreshold, @@ -1315,7 +1315,7 @@ contract Bridge is uint64 movedFundsSweepTxMaxTotalFee, uint32 movedFundsSweepTimeout, uint96 movedFundsSweepTimeoutSlashingAmount, - uint256 movedFundsSweepTimeoutNotifierRewardMultiplier + uint64 movedFundsSweepTimeoutNotifierRewardMultiplier ) external onlyGovernance { self.updateMovingFundsParameters( movingFundsTxMaxTotalFee, @@ -1395,10 +1395,10 @@ contract Bridge is /// - Fraud challenge defeat timeout must be greater than 0, /// - Fraud notifier reward multiplier must be in the range [0, 100]. function updateFraudParameters( - uint256 fraudChallengeDepositAmount, - uint256 fraudChallengeDefeatTimeout, + uint96 fraudChallengeDepositAmount, + uint32 fraudChallengeDefeatTimeout, uint96 fraudSlashingAmount, - uint256 fraudNotifierRewardMultiplier + uint32 fraudNotifierRewardMultiplier ) external onlyGovernance { self.updateFraudParameters( fraudChallengeDepositAmount, @@ -1607,9 +1607,9 @@ contract Bridge is uint64 redemptionDustThreshold, uint64 redemptionTreasuryFeeDivisor, uint64 redemptionTxMaxFee, - uint256 redemptionTimeout, + uint64 redemptionTimeout, uint96 redemptionTimeoutSlashingAmount, - uint256 redemptionTimeoutNotifierRewardMultiplier + uint64 redemptionTimeoutNotifierRewardMultiplier ) { redemptionDustThreshold = self.redemptionDustThreshold; @@ -1748,10 +1748,10 @@ contract Bridge is external view returns ( - uint256 fraudChallengeDepositAmount, - uint256 fraudChallengeDefeatTimeout, + uint96 fraudChallengeDepositAmount, + uint32 fraudChallengeDefeatTimeout, uint96 fraudSlashingAmount, - uint256 fraudNotifierRewardMultiplier + uint32 fraudNotifierRewardMultiplier ) { fraudChallengeDepositAmount = self.fraudChallengeDepositAmount; diff --git a/solidity/contracts/bridge/BridgeState.sol b/solidity/contracts/bridge/BridgeState.sol index 49087ad0f..2d42287dd 100644 --- a/solidity/contracts/bridge/BridgeState.sol +++ b/solidity/contracts/bridge/BridgeState.sol @@ -32,11 +32,11 @@ library BridgeState { Bank bank; // Bitcoin relay providing the current Bitcoin network difficulty. IRelay relay; - // ECDSA Wallet Registry contract handle. - EcdsaWalletRegistry ecdsaWalletRegistry; // The number of confirmations on the Bitcoin chain required to // successfully evaluate an SPV proof. - uint256 txProofDifficultyFactor; + uint96 txProofDifficultyFactor; + // ECDSA Wallet Registry contract handle. + EcdsaWalletRegistry ecdsaWalletRegistry; // Address where the deposit and redemption treasury fees will be sent // to. Treasury takes part in the operators rewarding process. address treasury; @@ -61,20 +61,10 @@ library BridgeState { // // This is a per-deposit input max fee for the sweep transaction. uint64 depositTxMaxFee; - // Collection of all revealed deposits indexed by - // `keccak256(fundingTxHash | fundingOutputIndex)`. - // The `fundingTxHash` is `bytes32` (ordered as in Bitcoin internally) - // and `fundingOutputIndex` an `uint32`. This mapping may contain valid - // and invalid deposits and the wallet is responsible for validating - // them before attempting to execute a sweep. - mapping(uint256 => Deposit.DepositRequest) deposits; - // Indicates if the vault with the given address is trusted or not. - // Depositors can route their revealed deposits only to trusted vaults - // and have trusted vaults notified about new deposits as soon as these - // deposits get swept. Vaults not trusted by the Bridge can still be - // used by Bank balance owners on their own responsibility - anyone can - // approve their Bank balance to any address. - mapping(address => bool) isVaultTrusted; + // Move movingFundsTxMaxTotalFee to the next storage slot for a more + // efficient variable layout + // slither-disable-next-line unused-state + bytes8 __depositAlignmentGap; // Maximum amount of the total BTC transaction fee that is acceptable in // a single moving funds transaction. // @@ -129,14 +119,7 @@ library BridgeState { // The percentage of the notifier reward from the staking contract // the notifier of a moved funds sweep timeout receives. The value is // in the range [0, 100]. - uint256 movedFundsSweepTimeoutNotifierRewardMultiplier; - // Collection of all moved funds sweep requests indexed by - // `keccak256(movingFundsTxHash | movingFundsOutputIndex)`. - // The `movingFundsTxHash` is `bytes32` (ordered as in Bitcoin - // internally) and `movingFundsOutputIndex` an `uint32`. Each entry - // is actually an UTXO representing the moved funds and is supposed - // to be swept with the current main UTXO of the recipient wallet. - mapping(uint256 => MovingFunds.MovedFundsSweepRequest) movedFundsSweepRequests; + uint64 movedFundsSweepTimeoutNotifierRewardMultiplier; // The minimal amount that can be requested for redemption. // Value of this parameter must take into account the value of // `redemptionTreasuryFeeDivisor` and `redemptionTxMaxFee` @@ -162,68 +145,33 @@ library BridgeState { // This is a per-redemption output max fee for the redemption // transaction. uint64 redemptionTxMaxFee; + // Move redemptionTimeout to the next storage slot for a more efficient + // variable layout + // slither-disable-next-line unused-state + bytes8 __redemptionAlignmentGap; // Time after which the redemption request can be reported as // timed out. It is counted from the moment when the redemption // request was created via `requestRedemption` call. Reported // timed out requests are cancelled and locked TBTC is returned // to the redeemer in full amount. - uint256 redemptionTimeout; + uint64 redemptionTimeout; // The amount of stake slashed from each member of a wallet for a // redemption timeout. uint96 redemptionTimeoutSlashingAmount; // The percentage of the notifier reward from the staking contract // the notifier of a redemption timeout receives. The value is in the // range [0, 100]. - uint256 redemptionTimeoutNotifierRewardMultiplier; - // Collection of all pending redemption requests indexed by - // redemption key built as - // `keccak256(walletPubKeyHash | redeemerOutputScript)`. - // The `walletPubKeyHash` is the 20-byte wallet's public key hash - // (computed using Bitcoin HASH160 over the compressed ECDSA - // public key) and `redeemerOutputScript` is a Bitcoin script - // (P2PKH, P2WPKH, P2SH or P2WSH) that will be used to lock - // redeemed BTC as requested by the redeemer. Requests are added - // to this mapping by the `requestRedemption` method (duplicates - // not allowed) and are removed by one of the following methods: - // - `submitRedemptionProof` in case the request was handled - // successfully, - // - `notifyRedemptionTimeout` in case the request was reported - // to be timed out. - mapping(uint256 => Redemption.RedemptionRequest) pendingRedemptions; - // Collection of all timed out redemptions requests indexed by - // redemption key built as - // `keccak256(walletPubKeyHash | redeemerOutputScript)`. The - // `walletPubKeyHash` is the 20-byte wallet's public key hash - // (computed using Bitcoin HASH160 over the compressed ECDSA - // public key) and `redeemerOutputScript` is the Bitcoin script - // (P2PKH, P2WPKH, P2SH or P2WSH) that is involved in the timed - // out request. Timed out requests are stored in this mapping to - // avoid slashing the wallets multiple times for the same timeout. - // Only one method can add to this mapping: - // - `notifyRedemptionTimeout` which puts the redemption key to this - // mapping basing on a timed out request stored previously in - // `pendingRedemptions` mapping. - mapping(uint256 => Redemption.RedemptionRequest) timedOutRedemptions; + uint64 redemptionTimeoutNotifierRewardMultiplier; // The amount of ETH in wei the party challenging the wallet for fraud // needs to deposit. - uint256 fraudChallengeDepositAmount; + uint96 fraudChallengeDepositAmount; // The amount of time the wallet has to defeat a fraud challenge. - uint256 fraudChallengeDefeatTimeout; + uint32 fraudChallengeDefeatTimeout; // The amount of stake slashed from each member of a wallet for a fraud. uint96 fraudSlashingAmount; // The percentage of the notifier reward from the staking contract // the notifier of a fraud receives. The value is in the range [0, 100]. - uint256 fraudNotifierRewardMultiplier; - // Collection of all submitted fraud challenges indexed by challenge - // key built as `keccak256(walletPublicKey|sighash)`. - mapping(uint256 => Fraud.FraudChallenge) fraudChallenges; - // Collection of main UTXOs that are honestly spent indexed by - // `keccak256(fundingTxHash | fundingOutputIndex)`. The `fundingTxHash` - // is `bytes32` (ordered as in Bitcoin internally) and - // `fundingOutputIndex` an `uint32`. A main UTXO is considered honestly - // spent if it was used as an input of a transaction that have been - // proven in the Bridge. - mapping(uint256 => bool) spentMainUTXOs; + uint32 fraudNotifierRewardMultiplier; // Determines how frequently a new wallet creation can be requested. // Value in seconds. uint32 walletCreationPeriod; @@ -259,6 +207,66 @@ library BridgeState { // of deposit fraud challenges. This value is in seconds and should be // greater than the deposit refund time plus some time margin. uint32 walletClosingPeriod; + // Collection of all revealed deposits indexed by + // `keccak256(fundingTxHash | fundingOutputIndex)`. + // The `fundingTxHash` is `bytes32` (ordered as in Bitcoin internally) + // and `fundingOutputIndex` an `uint32`. This mapping may contain valid + // and invalid deposits and the wallet is responsible for validating + // them before attempting to execute a sweep. + mapping(uint256 => Deposit.DepositRequest) deposits; + // Indicates if the vault with the given address is trusted or not. + // Depositors can route their revealed deposits only to trusted vaults + // and have trusted vaults notified about new deposits as soon as these + // deposits get swept. Vaults not trusted by the Bridge can still be + // used by Bank balance owners on their own responsibility - anyone can + // approve their Bank balance to any address. + mapping(address => bool) isVaultTrusted; + // Collection of all moved funds sweep requests indexed by + // `keccak256(movingFundsTxHash | movingFundsOutputIndex)`. + // The `movingFundsTxHash` is `bytes32` (ordered as in Bitcoin + // internally) and `movingFundsOutputIndex` an `uint32`. Each entry + // is actually an UTXO representing the moved funds and is supposed + // to be swept with the current main UTXO of the recipient wallet. + mapping(uint256 => MovingFunds.MovedFundsSweepRequest) movedFundsSweepRequests; + // Collection of all pending redemption requests indexed by + // redemption key built as + // `keccak256(walletPubKeyHash | redeemerOutputScript)`. + // The `walletPubKeyHash` is the 20-byte wallet's public key hash + // (computed using Bitcoin HASH160 over the compressed ECDSA + // public key) and `redeemerOutputScript` is a Bitcoin script + // (P2PKH, P2WPKH, P2SH or P2WSH) that will be used to lock + // redeemed BTC as requested by the redeemer. Requests are added + // to this mapping by the `requestRedemption` method (duplicates + // not allowed) and are removed by one of the following methods: + // - `submitRedemptionProof` in case the request was handled + // successfully, + // - `notifyRedemptionTimeout` in case the request was reported + // to be timed out. + mapping(uint256 => Redemption.RedemptionRequest) pendingRedemptions; + // Collection of all timed out redemptions requests indexed by + // redemption key built as + // `keccak256(walletPubKeyHash | redeemerOutputScript)`. The + // `walletPubKeyHash` is the 20-byte wallet's public key hash + // (computed using Bitcoin HASH160 over the compressed ECDSA + // public key) and `redeemerOutputScript` is the Bitcoin script + // (P2PKH, P2WPKH, P2SH or P2WSH) that is involved in the timed + // out request. Timed out requests are stored in this mapping to + // avoid slashing the wallets multiple times for the same timeout. + // Only one method can add to this mapping: + // - `notifyRedemptionTimeout` which puts the redemption key to this + // mapping basing on a timed out request stored previously in + // `pendingRedemptions` mapping. + mapping(uint256 => Redemption.RedemptionRequest) timedOutRedemptions; + // Collection of all submitted fraud challenges indexed by challenge + // key built as `keccak256(walletPublicKey|sighash)`. + mapping(uint256 => Fraud.FraudChallenge) fraudChallenges; + // Collection of main UTXOs that are honestly spent indexed by + // `keccak256(fundingTxHash | fundingOutputIndex)`. The `fundingTxHash` + // is `bytes32` (ordered as in Bitcoin internally) and + // `fundingOutputIndex` an `uint32`. A main UTXO is considered honestly + // spent if it was used as an input of a transaction that have been + // proven in the Bridge. + mapping(uint256 => bool) spentMainUTXOs; // Maps the 20-byte wallet public key hash (computed using Bitcoin // HASH160 over the compressed ECDSA public key) to the basic wallet // information like state and pending redemptions value. @@ -425,9 +433,9 @@ library BridgeState { uint64 _redemptionDustThreshold, uint64 _redemptionTreasuryFeeDivisor, uint64 _redemptionTxMaxFee, - uint256 _redemptionTimeout, + uint64 _redemptionTimeout, uint96 _redemptionTimeoutSlashingAmount, - uint256 _redemptionTimeoutNotifierRewardMultiplier + uint64 _redemptionTimeoutNotifierRewardMultiplier ) internal { require( _redemptionDustThreshold > self.movingFundsDustThreshold, @@ -546,7 +554,7 @@ library BridgeState { uint64 _movedFundsSweepTxMaxTotalFee, uint32 _movedFundsSweepTimeout, uint96 _movedFundsSweepTimeoutSlashingAmount, - uint256 _movedFundsSweepTimeoutNotifierRewardMultiplier + uint64 _movedFundsSweepTimeoutNotifierRewardMultiplier ) internal { require( _movingFundsTxMaxTotalFee > 0, @@ -709,10 +717,10 @@ library BridgeState { /// - Fraud notifier reward multiplier must be in the range [0, 100]. function updateFraudParameters( Storage storage self, - uint256 _fraudChallengeDepositAmount, - uint256 _fraudChallengeDefeatTimeout, + uint96 _fraudChallengeDepositAmount, + uint32 _fraudChallengeDefeatTimeout, uint96 _fraudSlashingAmount, - uint256 _fraudNotifierRewardMultiplier + uint32 _fraudNotifierRewardMultiplier ) internal { require( _fraudChallengeDefeatTimeout > 0, diff --git a/solidity/contracts/bridge/Redemption.sol b/solidity/contracts/bridge/Redemption.sol index c3f747ee9..804b43815 100644 --- a/solidity/contracts/bridge/Redemption.sol +++ b/solidity/contracts/bridge/Redemption.sol @@ -424,16 +424,14 @@ library Redemption { ); // Validate if redeemer output script is a correct standard type - // (P2PKH, P2WPKH, P2SH or P2WSH). This is done by building a stub - // output with 0 as value and using `BTCUtils.extractHash` on it. Such - // a function extracts the payload properly only from standard outputs - // so if it succeeds, we have a guarantee the redeemer output script - // is proper. Worth to note `extractHash` ignores the value at all - // so this is why we can use 0 safely. This way of validation is the - // same as in tBTC v1. - bytes memory redeemerOutputScriptPayload = abi - .encodePacked(bytes8(0), redeemerOutputScript) - .extractHash(); + // (P2PKH, P2WPKH, P2SH or P2WSH). This is done by using + // `BTCUtils.extractHashAt` on it. Such a function extracts the payload + // properly only from standard outputs so if it succeeds, we have a + // guarantee the redeemer output script is proper. The underlying way + // of validation is the same as in tBTC v1. + bytes memory redeemerOutputScriptPayload = redeemerOutputScript + .extractHashAt(0, redeemerOutputScript.length); + require( redeemerOutputScriptPayload.length > 0, "Redeemer output script must be a standard type" @@ -441,8 +439,8 @@ library Redemption { // Check if the redeemer output script payload does not point to the // wallet public key hash. require( - keccak256(abi.encodePacked(walletPubKeyHash)) != - keccak256(redeemerOutputScriptPayload), + redeemerOutputScriptPayload.length != 20 || + walletPubKeyHash != redeemerOutputScriptPayload.slice20(0), "Redeemer output script must not point to the wallet PKH" ); @@ -455,8 +453,9 @@ library Redemption { // and redeemer output script pair. That means there can be only one // request asking for redemption from the given wallet to the given // BTC script at the same time. - uint256 redemptionKey = uint256( - keccak256(abi.encodePacked(walletPubKeyHash, redeemerOutputScript)) + uint256 redemptionKey = getRedemptionKey( + walletPubKeyHash, + redeemerOutputScript ); // Check if given redemption key is not used by a pending redemption. @@ -497,6 +496,7 @@ library Redemption { uint32(block.timestamp) ); + // slither-disable-next-line reentrancy-events emit RedemptionRequested( walletPubKeyHash, redeemerOutputScript, @@ -674,7 +674,7 @@ library Redemption { /// must be validated using e.g. `BTCUtils.validateVout` function /// before it is passed here. /// @param walletPubKeyHash 20-byte public key hash (computed using Bitcoin - // HASH160 over the compressed ECDSA public key) of the wallet which + /// HASH160 over the compressed ECDSA public key) of the wallet which /// performed the redemption transaction. /// @return info Outcomes of the processing. function processRedemptionTxOutputs( @@ -722,7 +722,7 @@ library Redemption { // which matches the P2PKH structure as per: // https://en.bitcoin.it/wiki/Transaction#Pay-to-PubkeyHash bytes32 walletP2PKHScriptKeccak = keccak256( - abi.encodePacked(hex"1976a914", walletPubKeyHash, hex"88ac") + abi.encodePacked(BitcoinTx.makeP2PKHScript(walletPubKeyHash)) ); // The P2WPKH script has the byte format: <0x160014> <20-byte PKH>. // According to https://en.bitcoin.it/wiki/Script#Opcodes this translates to: @@ -732,7 +732,7 @@ library Redemption { // which matches the P2WPKH structure as per: // https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#P2WPKH bytes32 walletP2WPKHScriptKeccak = keccak256( - abi.encodePacked(hex"160014", walletPubKeyHash) + abi.encodePacked(BitcoinTx.makeP2WPKHScript(walletPubKeyHash)) ); return @@ -751,7 +751,7 @@ library Redemption { /// @notice Processes all outputs from the redemption transaction. Tries to /// identify output as a change output, pending redemption request - // or reported redemption. Reverts if one of the outputs cannot be + /// or reported redemption. Reverts if one of the outputs cannot be /// recognized properly. Marks each request as processed by removing /// them from `pendingRedemptions` mapping. /// @param redemptionTxOutputVector Bitcoin redemption transaction output @@ -759,7 +759,7 @@ library Redemption { /// must be validated using e.g. `BTCUtils.validateVout` function /// before it is passed here. /// @param walletPubKeyHash 20-byte public key hash (computed using Bitcoin - // HASH160 over the compressed ECDSA public key) of the wallet which + /// HASH160 over the compressed ECDSA public key) of the wallet which /// performed the redemption transaction. /// @param processInfo RedemptionTxOutputsProcessingInfo identifying output /// starting index, the number of outputs and possible wallet change @@ -783,24 +783,33 @@ library Redemption { // https://github.com/keep-network/tbtc-v2/issues/257 uint256 outputLength = redemptionTxOutputVector .determineOutputLengthAt(processInfo.outputStartingIndex); - bytes memory output = redemptionTxOutputVector.slice( - processInfo.outputStartingIndex, - outputLength - ); // Extract the value from given output. - uint64 outputValue = output.extractValue(); + uint64 outputValue = redemptionTxOutputVector.extractValueAt( + processInfo.outputStartingIndex + ); + + uint256 scriptLength = outputLength - 8; + // The output consists of an 8-byte value and a variable length - // script. To extract that script we slice the output starting from + // script. To hash that script we slice the output starting from // 9th byte until the end. - bytes memory outputScript = output.slice(8, output.length - 8); + + uint256 outputScriptStart = processInfo.outputStartingIndex + 8; + + bytes32 outputScriptHash; + /* solhint-disable-next-line no-inline-assembly */ + assembly { + outputScriptHash := keccak256( + add(redemptionTxOutputVector, add(outputScriptStart, 32)), + scriptLength + ) + } if ( resultInfo.changeValue == 0 && - (keccak256(outputScript) == - processInfo.walletP2PKHScriptKeccak || - keccak256(outputScript) == - processInfo.walletP2WPKHScriptKeccak) && + (outputScriptHash == processInfo.walletP2PKHScriptKeccak || + outputScriptHash == processInfo.walletP2WPKHScriptKeccak) && outputValue > 0 ) { // If we entered here, that means the change output with a @@ -815,8 +824,7 @@ library Redemption { uint64 treasuryFee ) = processNonChangeRedemptionTxOutput( self, - walletPubKeyHash, - outputScript, + _getRedemptionKey(walletPubKeyHash, outputScriptHash), outputValue ); resultInfo.totalBurnableValue += burnableValue; @@ -847,10 +855,7 @@ library Redemption { /// requested and reported timed-out redemption. /// This function also marks each pending request as processed by /// removing them from `pendingRedemptions` mapping. - /// @param walletPubKeyHash 20-byte public key hash (computed using Bitcoin - // HASH160 over the compressed ECDSA public key) of the wallet which - /// performed the redemption transaction. - /// @param outputScript Non-change output script to be processed. + /// @param redemptionKey Redemption key of the output being processed. /// @param outputValue Value of the output being processed. /// @return burnableValue The value burnable as a result of processing this /// single redemption output. This value needs to be summed up with @@ -864,17 +869,9 @@ library Redemption { /// redemption request. function processNonChangeRedemptionTxOutput( BridgeState.Storage storage self, - bytes20 walletPubKeyHash, - bytes memory outputScript, + uint256 redemptionKey, uint64 outputValue ) internal returns (uint64 burnableValue, uint64 treasuryFee) { - // This function should be called only if the given output is - // supposed to represent a redemption. Build the redemption key - // to perform that check. - uint256 redemptionKey = uint256( - keccak256(abi.encodePacked(walletPubKeyHash, outputScript)) - ); - if (self.pendingRedemptions[redemptionKey].requestedAt != 0) { // If we entered here, that means the output was identified // as a pending redemption request. @@ -975,9 +972,9 @@ library Redemption { bytes calldata redeemerOutputScript ) external { // Wallet state is validated in `notifyWalletRedemptionTimeout`. - - uint256 redemptionKey = uint256( - keccak256(abi.encodePacked(walletPubKeyHash, redeemerOutputScript)) + uint256 redemptionKey = getRedemptionKey( + walletPubKeyHash, + redeemerOutputScript ); Redemption.RedemptionRequest memory request = self.pendingRedemptions[ redemptionKey @@ -1017,4 +1014,43 @@ library Redemption { // Return the requested amount of tokens to the redeemer self.bank.transferBalance(request.redeemer, request.requestedAmount); } + + /// @notice Calculate redemption key without allocations. + /// @param walletPubKeyHash the pubkey hash of the wallet. + /// @param script the output script of the redemption. + /// @return The key = keccak256(keccak256(script), walletPubKeyHash). + function getRedemptionKey(bytes20 walletPubKeyHash, bytes memory script) + internal + pure + returns (uint256) + { + bytes32 scriptHash = keccak256(script); + uint256 key; + /* solhint-disable-next-line no-inline-assembly */ + assembly { + mstore(0, scriptHash) + mstore(32, walletPubKeyHash) + key := keccak256(0, 52) + } + return key; + } + + /// @notice Finish calculating redemption key without allocations. + /// @param walletPubKeyHash the pubkey hash of the wallet. + /// @param scriptHash the output script hash of the redemption. + /// @return The key = keccak256(scriptHash, walletPubKeyHash). + function _getRedemptionKey(bytes20 walletPubKeyHash, bytes32 scriptHash) + internal + pure + returns (uint256) + { + uint256 key; + /* solhint-disable-next-line no-inline-assembly */ + assembly { + mstore(0, scriptHash) + mstore(32, walletPubKeyHash) + key := keccak256(0, 52) + } + return key; + } } diff --git a/solidity/package.json b/solidity/package.json index 16eddad20..933d3f6ac 100644 --- a/solidity/package.json +++ b/solidity/package.json @@ -27,7 +27,7 @@ "prepublishOnly": "./scripts/prepare-artifacts.sh --network $npm_config_network" }, "dependencies": { - "@keep-network/bitcoin-spv-sol": "3.3.0-solc-0.8", + "@keep-network/bitcoin-spv-sol": "3.4.0-solc-0.8", "@keep-network/ecdsa": "development", "@keep-network/random-beacon": "development", "@keep-network/tbtc": ">1.1.2-dev <1.1.2-pre", diff --git a/solidity/test/bridge/Bridge.Frauds.test.ts b/solidity/test/bridge/Bridge.Frauds.test.ts index 24e867946..5e4a9196b 100644 --- a/solidity/test/bridge/Bridge.Frauds.test.ts +++ b/solidity/test/bridge/Bridge.Frauds.test.ts @@ -38,9 +38,9 @@ describe("Bridge - Fraud", () => { let bridge: Bridge & BridgeStub let fraudChallengeDepositAmount: BigNumber - let fraudChallengeDefeatTimeout: BigNumber + let fraudChallengeDefeatTimeout: number let fraudSlashingAmount: BigNumber - let fraudNotifierRewardMultiplier: BigNumber + let fraudNotifierRewardMultiplier: number before(async () => { // eslint-disable-next-line @typescript-eslint/no-extra-semi @@ -1961,7 +1961,7 @@ describe("Bridge - Fraud", () => { } ) - await increaseTime(fraudChallengeDefeatTimeout.sub(2)) + await increaseTime(fraudChallengeDefeatTimeout - 2) }) after(async () => { diff --git a/solidity/test/bridge/Bridge.Redemption.test.ts b/solidity/test/bridge/Bridge.Redemption.test.ts index b8e774604..3cf6bedeb 100644 --- a/solidity/test/bridge/Bridge.Redemption.test.ts +++ b/solidity/test/bridge/Bridge.Redemption.test.ts @@ -4374,8 +4374,11 @@ describe("Bridge - Redemption", () => { redeemerOutputScript: BytesLike ): string { return ethers.utils.solidityKeccak256( - ["bytes20", "bytes"], - [walletPubKeyHash, redeemerOutputScript] + ["bytes32", "bytes20"], + [ + ethers.utils.solidityKeccak256(["bytes"], [redeemerOutputScript]), + walletPubKeyHash, + ] ) } diff --git a/solidity/test/vault/TBTCVault.Redemption.test.ts b/solidity/test/vault/TBTCVault.Redemption.test.ts index 65e49ea25..0a00556c8 100644 --- a/solidity/test/vault/TBTCVault.Redemption.test.ts +++ b/solidity/test/vault/TBTCVault.Redemption.test.ts @@ -614,7 +614,10 @@ function buildRedemptionKey( redeemerOutputScript: BytesLike ): string { return ethers.utils.solidityKeccak256( - ["bytes20", "bytes"], - [walletPubKeyHash, redeemerOutputScript] + ["bytes32", "bytes20"], + [ + ethers.utils.solidityKeccak256(["bytes"], [redeemerOutputScript]), + walletPubKeyHash, + ] ) } diff --git a/solidity/yarn.lock b/solidity/yarn.lock index 12d5d86df..b8a3cbc5e 100644 --- a/solidity/yarn.lock +++ b/solidity/yarn.lock @@ -1503,10 +1503,10 @@ resolved "https://registry.yarnpkg.com/@humanwhocodes/object-schema/-/object-schema-1.2.1.tgz#b520529ec21d8e5945a1851dfd1c32e94e39ff45" integrity sha512-ZnQMnLV4e7hDlUvw8H+U8ASL02SS2Gn6+9Ac3wGGLIe7+je2AeAOxPY+izIPJDfFDb7eDjev0Us8MO1iFRN8hA== -"@keep-network/bitcoin-spv-sol@3.3.0-solc-0.8": - version "3.3.0-solc-0.8" - resolved "https://registry.yarnpkg.com/@keep-network/bitcoin-spv-sol/-/bitcoin-spv-sol-3.3.0-solc-0.8.tgz#5dfd552f9437e865038c1e97a2d27967c399dcbc" - integrity sha512-AG2fJqqniXVzwitHQcYQK5dZIyUPDo5xS1RZzW+LngcIuKA/f0cvTDEjWxgjf4D5MbStJNFWaCTtWwAJUCzsIw== +"@keep-network/bitcoin-spv-sol@3.4.0-solc-0.8": + version "3.4.0-solc-0.8" + resolved "https://registry.yarnpkg.com/@keep-network/bitcoin-spv-sol/-/bitcoin-spv-sol-3.4.0-solc-0.8.tgz#8b44c246ffab8ea993efe196f6bf385b1a3b84dc" + integrity sha512-KlpY9BbasyLvYXSS7dsJktgRChu/yjdFLOX8ldGA/pltLicCm/l0F4oqxL8wSws9XD12vq9x0B5qzPygVLB2TQ== "@keep-network/ecdsa@development": version "2.0.0-dev.46"