Thesis - tBTC and Keep

1 Executive Summary

In January 2020, Thesis asked us to conduct a security assessment of tBTC: a trust-minimized, redeemable, Bitcoin-backed ERC20 token. tBTC utilizes and builds on functionality provided by Summa and the Keep Network.

We performed this assessment from February 03 to March 27, 2020. The assessment primarily focused on tBTC alongside its associated components. The engagement was conducted by Martin Ortner and Alexander Wade over the course of twelve person-weeks.

In addition to the review of tBTC, a review was performed of the cryptographic constructions and algorithms used in the Keep Network. A complete report of this portion of the engagement can be found here.

1.1 Scope

We analyzed code located in the following repositories at the provided commits:

Third party dependencies not explicitly mentioned in the above list (e.g. summa-tx/relay-sol) were out of scope for the audit.

tBTC interacts with the Keep Network via customized interfaces from keep-network/keep-tecdsa, which itself uses keep-network/sortition-pools. The keep random beacon used for signer group election (keep-network/keep-core) builds on an implementation of BLS signatures on the altbn128 curve. The source code is located in five repositories with the following dependencies as seen from the tBTC solution:

• keep-network/tbtc
• summa-tx/bitcoin-spv
• keep-network/keep-tecdsa
  • keep-network/sortition-pools
  • keep-network/keep-core
• keep-network/keep-core (independent solution)

Together with the client, it was established that the main focus for the review would be the smart contracts in the listed repositories, with a secondary focus on reviewing the keep client (located in keep-core).

A complete list of files in scope can be found in the Appendix.

1.2 Objectives

Given the limited time available and ongoing development on some components in scope, we elected to begin with a top-down approach centered around tBTC as the focal point. We started by understanding the architecture and design of high-risk components first, before diving into various system components to verify security assumptions.

Our primary objectives were to:

1. Ensure that the system is implemented consistently with the intended functionality, and without unintended edge cases.
2. Identify known vulnerabilities particular to smart contract systems, as outlined in our Smart Contract Best Practices, and the Smart Contract Weakness Classification Registry.
3. Ensure that there is no way to break the TBTC-BTC peg and that it is as difficult as possible to abscond with deposited funds for the backing ECDSA keep.

We also sought opportunities to improve the quality of the code either by reducing the complexity, or improving clarity and readability.

1.3 Audit Log - Phase 1

The primary engagement (Feb 03 - Feb 28) was scheduled as follows:

Week 1

During the first week, our efforts were directed towards tBTC: understanding the intention of its design and how it uses bitcoin-spv to validate spv proofs and other Bitcoin transaction information. This involved defining key risk factors and potential vulnerabilities requiring further investigation. Key findings were shared with the client in an end-of-week sync meeting.

By the end of the first week, the tBTC codebase was modified from its initial audit commit to the revision v1-audit. The client also provided a frozen codebase for keep-network/keep-core. keep-network/keep-tecdsa was still undergoing changes.

Week 2

During the second week, we reviewed changes made to tBTC during the previous week. We also began a more detailed review of the tBTC codebase; in particular, tBTC Deposit flows and the investigation of potential vulnerabilities. Key findings were shared with the client in an end-of-week sync meeting and filed in the client repository where applicable. keep-network/keep-tecdsa was still undergoing changes by the end of week two.

The audit team informed the client that given the size and complexity of the audit there might not be enough time to cover all parts of the initial scope. Together with the client, it was determined that we would spend the next week finishing the review of tBTC Deposit flows before transitioning our review to keep-core.

Week 3

During the third week, we reviewed tBTC Deposit flows and started transitioning from tBTC to keep-core, maintaining a focus on the functionality of keep-core that was most relevant to tBTC.

The audit revision for the keep-tecdsa codebase was provided in the second half of the week and tagged as keep-tecdsa#v0.8.0. Additionally, the sortition-pools#v0.1.1 repository referenced by keep-tecdsa was added to the audit’s scope.

The cryptographic review that was planned to start this week had to be delayed due to availability problems with our cryptographer. The review of the keep client was temporarily set out of scope to ensure sufficient attention was given to the smart contracts. Key findings and questions were shared immediately via the client collaboration channel and discussed in an end-of-week sync meeting.

Week 4

During the fourth week, we focused on keep-core and the now frozen keep-tecdsa implementation. The week was kicked off by the client providing a walkthrough of the relevant code of keep-tecdsa. Key findings and questions were shared immediately via the client collaboration channel and discussed in an end-of-week sync meeting. The preliminary report outlining recommendations and findings was prepared towards the end of the week targeting delivery for the following Monday.

Two-week hiatus

A two-week hiatus allowing the client to address discussion points, recommendations, and issues found during the audit was planned from March 02 to March 13.

The engagement was scheduled to be continued for a final two-week review from March 16 to March 27.

1.4 Audit Log - Phase 2

The final phase of the engagement was scheduled as follows:

Week 1

During the first week after providing the initial report, we focused on continuing our efforts with keep-core and reviewing the feedback and fixes that were provided for the initial report. A secondary goal was to start reviewing the client implementations in keep-core. The client provided a high-level walkthrough of the keep client codebase and the audit team shared the sources for the tBTC state diagram (see Security - tBTC). The audit codebase was updated to the following revisions:

• tbtc: fbb2018c41456d19ec20eb28a17070ee2b10eb5d (noted above)
• keep-tecdsa: 2aab1f755e437d6e816c34a4fd354025cea5de3a (v0.10.0-rc)
• keep-core: 9f8b13fe54cc627548746d7e64b77d6aa50b94e1 (v0.11.0-rc) (provided on friday)
• sortition-pools: no update provided
• bitcoin-spv: no update provided

Week 2

During the second week, we continued with our focus on keep-core and started reviewing the client logic that is interacting with the smart contracts. The final report outlining recommendations and findings including client feedback and a review of provided fixes was prepared towards the end of the week targeting delivery for the following Monday. In addition to that the cryptographic review was finalized and prepared for the delivery on Monday.

2 Recommendations

During the course of our review, we identified a few possible improvements that are not security issues but can bring value to the developers and the people who want to interact with the system.

2.1 Perform extensive system simulation and integration tests prior to release

UPDATE: This recommendation has been addressed with the following statement: Manual system testing is currently underway, with automated testing to follow as we solidify structure. Automated system tests are captured at a high level in issues https://github.com/keep-network/tbtc/issues/339, https://github.com/keep-network/keep-ecdsa/issues/382, and https://github.com/keep-network/keep-core/issues/1556 for the respective components of the system.

Any highly-complex system benefits massively from integration testing. tBTC and the Keep Network are no exception: the two products tie together multiple different technologies (Bitcoin, Ethereum, sMPC, …) using mission-critical smart contracts. What’s more, the smart contracts in question implement strict timing windows for operations as well as steep penalties if those windows are missed.

Due in part to ongoing development on the codebase under review, no integration tests existed for the duration of this engagement. Although components of the system can be examined in relative isolation, the ability to review the system as a coherent whole is invaluable. By mimicking a production environment, integration testing helps uncover (among other things) simple issues that might otherwise only be discovered in production: misconfigurations, incorrect system-wide constants, and more.

Integration testing may also be used to simulate system behavior under a wide variety of network conditions. Due to this system’s heavy reliance on coordination between multiple off-chain networks, preparation for release should not be considered complete until the system is stress-tested in multiple non-ideal environments.

2.2 Consider reducing tBTC Deposit term or locking stake when in-use

UPDATE: This recommendation has been addressed with the following statement: We are moving to lock stakes during keep-ecdsa lifetimes if they exceed undelegation time in issue https://github.com/keep-network/keep-core/issues/1490. We consider this the more generic and future-proof of the recommended solutions.

A tBTC deposit reaches its term after 180 days. During this 180 day period, signers must maintain custody of the backing BTC. Should they attempt to commit fraud, they are punished in two ways:

1. Their bonds are seized. In the case of tBTC, this should be roughly equivalent to 150% of the value of the backing BTC, in ETH.
2. Their stake is slashed.

Token stake can be undelegated and withdrawn in only 90 days. Although the security model of tBTC is mostly reliant on the seizing of signer bonds (rather than stake slashing), this will almost certainly be abused if a signer acts maliciously.

Consider either disallowing undelegation when a signer is a member of an active keep, or reducing tBTC deposit term to under 90 days.

2.3 Disallow overfunding of tBTC Deposits

UPDATE: This recommendation has been addressed with the following statement: Unfortunately, there isn’t a simple way to disable mistaken funding in an economically sound way. We are considering adding a purely social, unguaranteed “please return my UTXO” function for funders, which would have no repercussions for signers but would return their bond and, request that they return the UTXO. Such a solution would have no on-chain enforcement. This possibility is tracked in issue https://github.com/keep-network/tbtc/issues/550.

All deposits have an associated lot size, or amount of BTC. When initially funding a deposit, the funder is expected to send the entire lot-size-worth of BTC in a single transaction. In other words, funding a deposit over the course of two transactions is not supported and will result in a loss of funds.

However, overfunding a deposit is allowed: a funder can send more than the lot-size-worth of BTC to the deposit address. Consider disallowing this behavior, as it encourages users to circumvent the provided UI.

2.4 Improve error handling in bitcoin-spv

Several of our findings detailed potential error states in bitcoin-spv. Overall, the bitcoin-spv libraries tend to “fail silently,” returning “garbage” values when error states are achieved, rather than reverting. This tendency places a larger burden on the library’s users, requiring them to understand more about the library’s function to use it safely. While this is a valid expectation, it is typically not realistic.

Additionally, implementing error handling in bitcoin-spv will allow for more negative test cases, improving overall code quality and test coverage.

2.5 Simplify the deposit flow

UPDATE: This recommendation has been addressed with the following statement: The deposit flow has been simplified as part of recommendation 2.6, and issue 5.9 below; see those for more information.

Deposit flow is highly complicated, so simplifying it wherever possible should be a priority. If possible, reduce the number states and transitions a TDT tracked deposit can be in. This also reduces the number of interactions with the deposit and therefore saves users gas. Avoid adding states for the sole purpose of tracking what path a deposit came from and deduplicate redundant states (LIQUIDATION).

2.6 Remove funding fraud states in Deposit

UPDATE: This recommendation has been addressed with the following statement: This refactor was done while addressing issue 5.9 below in GitHub issue https://github.com/keep-network/tbtc/issues/494. The details of the change differ slightly from this recommendation: rather than use the same fraud path used in active deposits, fraud during funding always sends the full signer bond to the funder.

Deposit flow includes two types of fraud-proof. The first is submitted during the AWAITING_BTC_FUNDING_PROOF state and punishes signers for fraud committed during the funding stage. The second is submitted during most other states and punishes signers for fraud committed outside the funding stage.

Because the punishment differs across these two fraud-proof submission methods, there is occasionally incentive to commit fraud in the funding stage, advance the deposit state to post-funding, and submit a fraud-proof using the post-funding fraud-proof functions. In particular, post-funding fraud-proofs award the fraud-proof initiator with a cut of the bonds seized from signers, whereas funding fraud-proofs do not.

Rather than include additional complexity with different incentives, merge the two fraud submission methods and make them available throughout Deposit flow.

2.7 Improve signer bond seize efficiency

UPDATE: This recommendation has been addressed with the following statement: We are considering pull payments across the system as part of issue https://github.com/keep-network/tbtc/issues/551, which is focused on the broader problem of ensuring an incorrect payment recipient cannot prevent disbursals and state transitions from completing.

BondedECDSAKeep.seizeSignerBonds iterates over a keep’s members, queries each member’s bond amount, and seizes each member’s bond individually - netting 2 * members.length external calls:

    function seizeSignerBonds() external onlyOwner onlyWhenActive {
        markAsClosed();

        for (uint256 i = 0; i < members.length; i++) {
            uint256 amount = keepBonding.bondAmount(
                members[i],
                address(this),
                uint256(address(this))
            );

            keepBonding.seizeBond(
                members[i],
                uint256(address(this)),
                amount,
                address(uint160(owner))
            );
        }
    }

Additionally, keepBonding.seizeBond makes another external call to address payable destination for a total of 3 * members.length calls. If any of these fail, the entire call fails and signer bonds cannot be seized.

Because this function is so crucial to the security properties of tBTC, consider switching to a pull payment system.

2.8 Improve TBTCSystem lot size updates

UPDATE: This recommendation has been addressed with the following statement: We are intending to make the remainder of this change, tracked in issue https://github.com/keep-network/tbtc/issues/552.

TBTCSystem currently tracks allowed BTC lot sizes in an array, lotSizesSatoshis. Tracking lot sizes with an array is highly inefficient, as updates and queries require costly iteration.

Remove the array and replace it with a mapping from uint lotSize => bool supported. Use a setter function to allow updates of supported lot sizes. Use a getter function to query currently-allowed lot sizes.

Additionally, the setter function should include strict checks to determine whether a lot size is valid:

• 1 BTC should always be allowed
• 0 should not be allowed
• A reasonable minimum should be enforced. One potential option is 0.001 BTC
• A reasonable maximum should be enforced. One potential option is 10 BTC.

2.9 Explicitly track current and previous state/flow instead of deriving it from side-effects

UPDATE: This recommendation has been addressed with the following statement: Of the two listed examples, only one is valid (purchaseSignerBondsAtAuction uses tdtHolder solely to implement the mechanic of “sending TBTC to the vending machine should be equivalent to burning that TBTC” efficiently; the two branches are effectively the same, except that the vending machine does not have a built-in way to handle an incoming token transfer and implement the “burn” mechanic itself). The second listed note, startSignerFraudLiquidation’s use of the auction amount to determine that it originated in redemption, is being tracked as https://github.com/keep-network/tbtc/issues/553.

We recommend explicitly tracking the origin flow/state or even the transition history in the deposit instead of deriving it from side-effects or assuming other variables contain certain values.

• purchaseSignerBondsAtAuction uses the tdtHolder address to distinguish whether a liquidation was started for an active deposit or not.

if(tdtHolder == _d.VendingMachine){
    _tbtcToken.burnFrom(msg.sender, lotSizeTbtc);  // burn minimal amount to cover size
}
else{
    _tbtcToken.transferFrom(msg.sender, tdtHolder, lotSizeTbtc);
}

• startSignerFraudLiquidation derives the origin flow from the acutionTTBTCAmount()

if (_d.auctionTBTCAmount() == 0) {
    // we came from the redemption flow
    _d.setLiquidated();
    _d.redeemerAddress.transfer(_seized);
    _d.logLiquidated();
    return;
}

2.10 Consider emitting events for security-critical actions

UPDATE: This recommendation has been addressed with the following statement: All security-critical actions now emit events and are implemented in two phases with a delay (see 2.15 below).

Events can be an easy way to produce an audit trail for security-critical actions performed on the contract system. Furthermore, these events can be used to build a custom monitoring and intrusion detection system that alarms the operators of a potential upcoming attack campaign or misuse of the system and may allow cutting reaction time ensuring the safety of the system.

2.11 Review all comments

UPDATE: This recommendation has been addressed with the following statement: This is being tracked as issue https://github.com/keep-network/tbtc/issues/554.

As developers, we often forget to update comments when making changes because comments do not affect us immediately. However, the presence of TODO’s in code implies that the codebase is not yet ready for production. This can be an oversight or a sign that code is still undergoing changes.

Make sure to review all of the comments after the code was frozen.

2.12 Review and update the specification and documentation

UPDATE: This recommendation has been addressed with the following statement: This is being tracked alongside 2.11 in https://github.com/keep-network/tbtc/issues/554.

During an audit, we typically verify that a system complies with its design and specification documents. Our review of tBTC uncovered multiple inaccuracies between the code and details in the documentation of both tbtc and keep/random-beacon. Most of these inaccuracies likely stem from recent changes to the codebase that have not yet been updated in the documentation.

We have shared a list of inconsistencies for tBTC with the client, some of which were:

• non-existent state SIGNER_MARGIN_CALLED mentioned in the specification
• transitions to FRAUD_LIQUIDATION_IN_PROGRESS

The specification states that provideECDSAFraudProof and provideSPVFraudProof transitions from the following states, which is inconsistent with the implementation:

from AWAITING_SIGNER_SETUP
AWAITING_BTC_FUNDING_PROOF
ACTIVE
AWAITING_WITHDRAWAL_SIGNATURE
AWAITING_WITHDRAWAL_PROOF
SIGNER_MARGIN_CALLED
to
FRAUD_LIQUIDATION_IN_PROGRESS

• the state SIGNER_MARGIN_CALLED does not exist

• the fraud-proof methods cannot be used to transitions from AWAITING_SIGNER_SETUP, AWAITING_BTC_FUNDING_PROOF to FRAUD_LIQUIDATION_IN_PROGRESS

• LIQUIDATION_IN_PROGRESS is reachable via fraud-proof

  The specification mentions that in the redemption flow the state LIQUIDATION_IN_PROGRESS is reachable via an ECDSA or BTC fraud-proof.

Reachable exterior states
LIQUIDATION_IN_PROGRESS
via an ECDSA or BTC fraud-proof
via a state timeout

However, the correct state after providing a fraud-proof from redemption should be FRAUD_LIQUIDATION_IN_PROGRESS.

2.13 Review all constants and avoid changing them for testing purposes

UPDATE: This recommendation has been addressed with the following statement: The nature of testing and the time frames used in the system is such that time constants are hard not to adjust for testing purposes; that said, the configuration of test-specific constants to only be set in stub contracts dedicated to testing is being taken up as part of issue https://github.com/keep-network/tbtc/issues/555.

Multiple system constants have been tuned for testing and were not reset to production values for the frozen audit commits. We strongly recommend avoiding permanently changing system variables for testing. Instead, test classes and mock contracts should override constants where applicable.

Note, too, that changing important system variables for testing creates a gap where the actual system configuration for production might not receive as much testing as an artificial test scenario.

uint256 public constant DEPOSIT_TERM_LENGTH = 180 * 24 * 60 * 60; // 180 days in seconds
uint256 public constant TX_PROOF_DIFFICULTY_FACTOR = 1; // TODO: decreased for testing, original value: `6`

2.14 Avoid overlapping phases when using timed periods

UPDATE: This recommendation has been addressed with https://github.com/keep-network/keep-core/issues/1443.

Where possible, states should be clearly distinguished from each other with no overlap. It should be avoided that objects can be in two states at the same time.

For example, in keep-core/TokenStaking, stake can be in the initializationPeriod, active, or active_and_waiting_for_undelegation.

cancelStake checks that the stake is within the initializationPeriod like so:

require(
    block.number <= operators[_operator].createdAt.add(initializationPeriod),
    "Initialization period is over"
);

eligibleStake verifies a stake is not in initializationPeriod like so:

bool isActive = block.number >= operator.createdAt.add(initializationPeriod);

In the case when block.number == operators[_operator].createdAt.add(initializationPeriod), stake creation time satisfies both of these conditions and is in two states at the same time.

2.15 Follow best practices when upgrading and changing system variables

UPDATE: This recommendation has been addressed with the following statement: Two-step timelocked upgrades were implemented as part of issues https://github.com/keep-network/tbtc/issues/493, https://github.com/keep-network/keep-ecdsa/issues/296, and https://github.com/keep-network/keep-core/issues/1423.

Changing the behavior of system components via upgrading the smart contracts or modification of shared settings should be transparent and predictable for users and allow them to act on forthcoming changes. Changes that take effect immediately may allow for manipulation opportunities for the party executing the change by front-running other transactions or by setting and resetting parameters for their own profit.

We recommend implementing a time-lock that informs users of planned changes and gives them sufficient time to react to an unwanted change. It is also recommended to use a multisig contract or other transparent governance mechanisms to initiate changes.

2.16 Initialization of proxy contracts

UPDATE: This recommendation has been addressed with the following statement: This recommendation was implemented as part of issues https://github.com/keep-network/keep-ecdsa/issues/296 and https://github.com/keep-network/keep-core/issues/1423.

Ensure that implementations for proxy contracts are either initialized in the constructor when being deployed or the initialization method (and storage-changing functionality) is protected from being called by anyone. Consider rejecting calls to state reading/writing methods for contracts that are pending initialization.

It should generally be technically enforced that contracts are initialized in the same transaction as they are deployed or upgraded. This is especially true if the initialization method cannot be protected and may be called by third parties.

Ensure the existing storage layout does not change when upgrading the implementation.

2.17 Keep group should prove that they are capable of signing a message

UPDATE: This recommendation has been addressed with the following statement: Note that funders are now refunded in case of keep signature setup failure as part of https://github.com/keep-network/tbtc/issues/495, and all remaining bonds are returned to the signers. The remaining issue, of ensuring that the signers can indeed sign with the key they are publishing, does not significantly differ from one of the signers being unavailable when a signature is requested from them, which is a scenario that is already handled by the system.

When a new keep is formed members start the DKG process and prove to the BondedECDSAKeep contract that they are capable of participating in signing requests by submitting the public key to the contract. The fact that the keep formation succeeded is visible to consumers of the keep by checking the contracts publicKey state variable which is only set if all members confirmed the pubkey.

While this proves that all members submitted the same on-chain observable value pubKey, it does not prove that the group is capable of signing data. For example, once members confirm the public key, the funder in tBTC is able to move her deposit to the next state, sending BTC to the keep address. Given that funds are at risk we would recommend ensuring the funder (or any other consumer of the keep) that the keep group is indeed willing and capable of fulfilling signing requests. This could be accomplished by providing a message (e.g. keep owner address) signed for the funder.

The process of forming a keep group may also be susceptible to a minority attack where at least one member blocks the setup of the keep group by not confirming or confirming a wrong pubkey. The keep cannot recover from this attack, the member submitting the wrong pubKey cannot re-submit a valid one again, the key generation will time out without a pubKey being set for the keep. The keep is not activated and unable to perform signing requests. There are only three ways to proceed:

• no action, bonds stay locked in the keep
• signer bonds are seized by the owner (deposit)
• keep is closed by the owner (deposit) in which case the member bonds are released

In tBTC one would call notifySignerSetupFailure on the deposit now to terminate it. In any case the funder the tBTC side loses the initial payment to the keep because the keep setup was blocked and the signer bonds are not seized. On the keep side of things, all keep members bonds are locked up as the tBTC deposit does not close the keep freeing the bonds. This scenario presents a case where by investing one member bond, this member can cause the tBTC funder and the rest of the keep members to lose funds (keep payment, bonds).

There is no recommended mitigation for this other than ensuring that keeps never fail to setup. The tBTC funder cannot be reimbursed for their loss as creating a keep incurs costs. The honest majority of keep members could be reimbursed but that might open up other attack vectors. A possible solution could be to dynamically match members to a keep until all of them were able to prove that they are capable of signing for the keep but that might require major changes to the system.

2.18 Improve Input validation

UPDATE: This recommendation has been addressed with the following statement: These issues have been addressed where reasonable and feasible across the codebases.

Input validation checks should be explicit and well documented as part of the code’s documentation. This is to make sure that smart-contracts are robust against erroneous inputs and reduce the potential attack surface for exploitation.

It is good practice to verify the method’s input as early as possible and only perform further actions if the validation succeeds. Methods can be split into an external or public API that performs initial checks and subsequently calls an internal method that performs the action.

There is a lack of input validation throughout the codebases under audit. For example, during the audit, we suggested implementing more restrict input validation for bitcoin-spv to make error conditions more explicit. Methods receiving addresses should check whether the address is valid before storing it especially if it cannot be changed afterward (optionally checking EXTCODESIZE). Known upper and lower bounds for variables must be enforced. For example, it should not be allowed to create a keep group of size zero, zero amount stake or withdraw zero eth from the staking contract. We recommend designing methods to explicitly fail early for unexpected input to allow better error handling and reduce the potential attack surface. The Checks-Effects-Interactions pattern should be used for methods and implicit error handling should be avoided (e.g. method throws because of out of bounds access in array).

2.19 Client - Add Security Linting step to CI pipeline

UPDATE: This recommendation has been addressed with the following statement: The tbtc codebase has had linting installed since very early on; Solidity linting was added to keep-ecdsa as part of https://github.com/keep-network/keep-ecdsa/issues/42. Go linting has been running as part of pre-commit hooks across both keep-ecdsa and keep-core, but will be added to CI as part of https://github.com/keep-network/keep-ecdsa/issues/358 and https://github.com/keep-network/keep-core/issues/1551.

It is recommended to add a security-linting step to keep-core and keep-ecdsa making sure that minimum security requirements are enforced for every changeset.

The golangci-lint project is a convenient linter-aggregator that can be used for this purpose.

keep-core

pkg/beacon/relay/group/message_filter.go:86:2: S1008: should use 'return <expr>' instead of 'if <expr> { return <bool> }; return <bool>' (gosimple)
	if message.SenderID() == memberIndex {
	^
cmd/start.go:23:2: `bootstrapFlag` is unused (varcheck)
	bootstrapFlag = "bootstrap"
	^
pkg/chain/ethereum/utility.go:49:5: ineffectual assignment to `err` (ineffassign)
	_, err = euc.keepRandomBeaconServiceContract.WatchRelayEntryRequested(
	   ^
pkg/chain/ethereum/lib.go:53:6: `errorCallback` is unused (deadcode)
type errorCallback func(err error) (eout error)
     ^
pkg/chain/ethereum/lib.go:56:6: `sum256` is unused (deadcode)
func sum256(data []byte) (digest [32]byte) {
     ^
pkg/beacon/relay/dkg/result/signing.go:12:6: `dkgResultSignature` is unused (deadcode)
type dkgResultSignature = []byte 
     ^
config/config.go:15:7: G101: Potential hardcoded credentials (gosec)
const passwordEnvVariable = "KEEP_ETHEREUM_PASSWORD"
      ^
pkg/beacon/relay/gjkr/gjkr.go:22:29: Error return value of `channel.RegisterUnmarshaler` is not checked (errcheck)
	channel.RegisterUnmarshaler(func() net.TaggedUnmarshaler { 
	                           ^
pkg/beacon/relay/gjkr/gjkr.go:25:29: Error return value of `channel.RegisterUnmarshaler` is not checked (errcheck)
	channel.RegisterUnmarshaler(func() net.TaggedUnmarshaler {
	                           ^
pkg/beacon/relay/gjkr/gjkr.go:28:29: Error return value of `channel.RegisterUnmarshaler` is not checked (errcheck)
	channel.RegisterUnmarshaler(func() net.TaggedUnmarshaler {
	                           ^
pkg/beacon/relay/gjkr/protocol.go:83:37: Error return value of `sm.evidenceLog.PutEphemeralMessage` is not checked (errcheck)
		sm.evidenceLog.PutEphemeralMessage(ephemeralPubKeyMessage)
		                                  ^
pkg/beacon/relay/gjkr/protocol.go:312:39: Error return value of `cvm.evidenceLog.PutPeerSharesMessage` is not checked (errcheck)
		cvm.evidenceLog.PutPeerSharesMessage(sharesMessage)
		                                    ^
pkg/net/libp2p/channel.go:330:35: Error return value of `c.pubsub.UnregisterTopicValidator` is not checked (errcheck)
	c.pubsub.UnregisterTopicValidator(c.name) 
	                                 ^
pkg/chain/ethereum/lib.go:58:9: Error return value of `h.Write` is not checked (errcheck)
	h.Write(data)
	       ^
pkg/chain/ethereum/utility.go:36:15: Error return value of `promise.Fail` is not checked (errcheck)
		promise.Fail(err)
		            ^
pkg/chain/ethereum/utility.go:41:15: Error return value of `promise.Fail` is not checked (errcheck)
		promise.Fail(err)
		            ^
pkg/chain/ethereum/utility.go:56:64: Error return value of `euc.keepRandomBeaconServiceContract.WatchRelayEntryGenerated` is not checked (errcheck)
			euc.keepRandomBeaconServiceContract.WatchRelayEntryGenerated(
			                                                            ^
pkg/chain/ethereum/utility.go:58:21: Error return value of `promise.Fulfill` is not checked (errcheck)
					promise.Fulfill(&event.EntryGenerated{
					               ^
pkg/chain/ethereum/utility.go:76:15: Error return value of `promise.Fail` is not checked (errcheck)
		promise.Fail(err)
		            ^
pkg/internal/dkgtest/dkgtest.go:104:45: Error return value of `(github.com/keep-network/keep-core/pkg/beacon/relay/chain.DistributedKeyGenerationInterface).OnDKGResultSubmitted` is not checked (errcheck)
	chain.ThresholdRelay().OnDKGResultSubmitted(
	                                           ^
pkg/internal/entrytest/entrytest.go:110:46: Error return value of `(github.com/keep-network/keep-core/pkg/beacon/relay/chain.RelayEntryInterface).OnRelayEntrySubmitted` is not checked (errcheck)
	chain.ThresholdRelay().OnRelayEntrySubmitted( 
	                                            ^
pkg/beacon/beacon.go:65:34: Error return value of `relayChain.OnRelayEntryRequested` is not checked (errcheck)
	relayChain.OnRelayEntryRequested(func(request *event.Request) {
	                                ^
pkg/beacon/beacon.go:91:36: Error return value of `relayChain.OnGroupSelectionStarted` is not checked (errcheck)
	relayChain.OnGroupSelectionStarted(func(event *event.GroupSelectionStart) { 
	                                  ^
pkg/beacon/beacon.go:130:30: Error return value of `relayChain.OnGroupRegistered` is not checked (errcheck)
	relayChain.OnGroupRegistered(func(registration *event.GroupRegistration) { 
	                            ^
cmd/network.go:78:26: Error return value of `stakeMonitor.StakeTokens` is not checked (errcheck)
	stakeMonitor.StakeTokens(key.NetworkPubKeyToEthAddress(
	                        ^
cmd/network.go:81:26: Error return value of `stakeMonitor.StakeTokens` is not checked (errcheck)
	stakeMonitor.StakeTokens(key.NetworkPubKeyToEthAddress( 
	                        ^
pkg/beacon/relay/node.go:21:2: `mutex` is unused (structcheck)
	mutex sync.Mutex
	^
pkg/net/libp2p/channel.go:45:17: SA1019: peerstore.Peerstore is deprecated: use github.com/libp2p/go-libp2p-core/peerstore.Peerstore instead.  (staticcheck)
	peerStore      peerstore.Peerstore 
	               ^
pkg/net/libp2p/channel.go:184:9: SA1019: c.pubsub.Publish is deprecated: use pubsub.Join() and topic.Publish() instead  (staticcheck)
	return c.pubsub.Publish(c.name, messageBytes)
	       ^
pkg/net/libp2p/channel_manager.go:25:12: SA1019: peerstore.Peerstore is deprecated: use github.com/libp2p/go-libp2p-core/peerstore.Peerstore instead.  (staticcheck)
	peerStore peerstore.Peerstore 
	          ^
pkg/net/libp2p/channel_manager.go:96:14: SA1019: cm.pubsub.Subscribe is deprecated: use pubsub.Join() and topic.Subscribe() instead  (staticcheck)
	sub, err := cm.pubsub.Subscribe(name) 
	            ^
pkg/net/libp2p/libp2p.go:159:17: SA1019: peer.IDB58Decode is deprecated: Use Decode.  (staticcheck)
	peerID, err := peer.IDB58Decode(connectedPeer) 
	               ^
pkg/net/libp2p/libp2p.go:181:17: SA1019: peer.IDB58Decode is deprecated: Use Decode.  (staticcheck)
	peerID, err := peer.IDB58Decode(peerHash) 
	               ^
pkg/net/libp2p/libp2p.go:435:51: SA1019: peerstore.PeerInfo is deprecated: use github.com/libp2p/go-libp2p-core/peer.Info instead.  (staticcheck)
func extractMultiAddrFromPeers(peers []string) ([]peerstore.PeerInfo, error) { 
                                                  ^
pkg/net/libp2p/libp2p.go:436:18: SA1019: peerstore.PeerInfo is deprecated: use github.com/libp2p/go-libp2p-core/peer.Info instead.  (staticcheck)
	var peerInfos []peerstore.PeerInfo
	                ^
pkg/net/libp2p/libp2p.go:443:20: SA1019: peerstore.InfoFromP2pAddr is deprecated: use github.com/libp2p/go-libp2p-core/peer.AddrInfoFromP2pAddr instead.  (staticcheck)
		peerInfo, err := peerstore.InfoFromP2pAddr(ipfsaddr)
		                 ^
pkg/net/libp2p/unicast_channel_manager.go:141:21: SA1019: peer.IDB58Decode is deprecated: Use Decode.  (staticcheck)
	remotePeer, err := peer.IDB58Decode(peerID.String())
	                   ^
pkg/beacon/relay/node.go:128:2: S1023: redundant `return` statement (gosimple)
	return 
	^
pkg/beacon/relay/node.go:68:6: S1004: should use bytes.Equal(selectedStaker, n.Staker.Address()) instead (gosimple)
		if bytes.Compare(selectedStaker, n.Staker.Address()) == 0 {
		   ^
pkg/beacon/relay/dkg/result/states.go:76:10: S1004: should use bytes.Equal(phaseMessage.publicKey, msg.SenderPublicKey()) instead (gosimple)
		return bytes.Compare(phaseMessage.publicKey, msg.SenderPublicKey()) == 0
		       ^
pkg/net/watchtower/watchtower.go:53:2: S1005: should write `checking := g.peerCrossList[peer]` instead of `checking, _ := g.peerCrossList[peer]` (gosimple)
	checking, _ := g.peerCrossList[peer]
	^
cmd/relay.go:81:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
	select {
	^
cmd/start.go:125:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
	select {
	^
pkg/chain/ethereum/ethereum.go:215:3: S1000: should use for range instead of for { select {} } (gosimple)
		for {
		^
pkg/chain/ethereum/ethereum.go:420:3: S1000: should use for range instead of for { select {} } (gosimple)
		for {
		^
pkg/beacon/relay/group/group.go:139:17: func `(*Group).isThresholdSatisfied` is unused (unused)
pkg/beacon/relay/group/group.go:132:17: func `(*Group).eliminatedMembersCount` is unused (unused)

keep-ecdsa

pkg/chain/eth/local/local.go:62:15: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
	handlerID := rand.Int()
	             ^
pkg/chain/eth/local/local.go:83:15: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
	handlerID := rand.Int()
	             ^
internal/config/config.go:13:7: G101: Potential hardcoded credentials (gosec)
const passwordEnvVariable = "KEEP_ETHEREUM_PASSWORD"
      ^
pkg/ecdsa/tss/message.go:43:38: Error return value of `broadcastChannel.RegisterUnmarshaler` is not checked (errcheck)
	broadcastChannel.RegisterUnmarshaler(func() net.TaggedUnmarshaler {
	                                    ^
pkg/ecdsa/tss/message.go:46:38: Error return value of `broadcastChannel.RegisterUnmarshaler` is not checked (errcheck)
	broadcastChannel.RegisterUnmarshaler(func() net.TaggedUnmarshaler {
	                                    ^
pkg/ecdsa/tss/message.go:49:38: Error return value of `broadcastChannel.RegisterUnmarshaler` is not checked (errcheck)
	broadcastChannel.RegisterUnmarshaler(func() net.TaggedUnmarshaler {
	                                    ^
pkg/ecdsa/tss/network.go:266:14: Error return value of `b.broadcast` is not checked (errcheck)
		b.broadcast(ctx, protocolMessage)
		           ^
pkg/ecdsa/tss/network.go:279:12: Error return value of `b.sendTo` is not checked (errcheck)
			b.sendTo(destinationTransportID, protocolMessage)
			        ^
pkg/ecdsa/tss/network.go:294:26: Error return value of `broadcastChannel.Send` is not checked (errcheck)
	if broadcastChannel.Send(ctx, msg); err != nil {
	                        ^
pkg/client/client.go:105:40: Error return value of `ethereumChain.OnBondedECDSAKeepCreated` is not checked (errcheck)
	ethereumChain.OnBondedECDSAKeepCreated(func(event *eth.BondedECDSAKeepCreatedEvent) {
	                                      ^
pkg/node/node.go:136:2: lostcancel: the monitoringCancel function is not used on all paths (possible context leak) (govet)
	monitoringCtx, monitoringCancel := context.WithTimeout(
	^
pkg/node/node.go:157:2: lostcancel: this return statement may be reached without using the monitoringCancel var defined on line 136 (govet)
	return signer, nil
	^
cmd/start.go:163:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
	select {
	^
pkg/ecdsa/tss/protocol_announce.go:77:3: S1023: redundant `return` statement (gosimple)
		return
		^
pkg/ecdsa/tss/protocol_ready.go:82:3: S1023: redundant `return` statement (gosimple)
		return
		^

2.20 Client - Ensure nodes cannot be booted off the network

UPDATE: This recommendation has been addressed with the following statement: Fuzzing is being tracked alongside broader system testing in https://github.com/keep-network/keep-core/issues/1556 and https://github.com/keep-network/keep-ecdsa/issues/382.

A major threat to the system is that an actor may be able to boot nodes off the network by causing a panic while interacting with them. Someone who is able to permanently or temporarily reduce the amount of responsible nodes in the system may be able directly harm the network or turn things to their favor. The threat scenario is somewhat similar to the one the go-ethereum project is facing. We therefore recommend to design the software with security zones in mind, ensuring that sub-routines that are handling untrusted input cannot terminate the application e.g. because a panic condition has been triggered. We also recommend to set-up a fuzz-testing instance with go-fuzz especially for parts that are parsing/handling untrusted input, in an effort to find yet uncaught potentially triggerable panic conditions. Where feasible, we recommend to safeguard critical functionality that is handling untrusted data by trying to recover from panic events instead of terminating the application while still logging the error condition.

2.21 Review the Code Quality recommendations in Appendix 1

UPDATE: This recommendation has been addressed with the following statement: See remarks in Appendix 1 for more.

Other comments related to readability and best practices are listed in Appendix 1

3 System Overview

This section describes the top-level contracts, their inheritance structure, actors, permissions and contract interactions of the initial system under audit, not including fundamental changes the system has undergone after providing the initial report. Please refer to Section 4 - Security Specification for a security-centric view on the system.

3.1 tBTC

Inheritance Structure (without usingFor)

Call Graph

System Overview

The following components are referencing contracts in one of the other repositories that are in scope for the audit:

Deposits

• main entry point for users to mint TBTC by creating a TBTCDepositToken tracked deposit.
• deposits can be in various states starting with a funding flow that ultimately reached the active state, handling, and reporting of timeouts and frauds as well as undercollateralization. A deposit can also be redeemed or liquidated.

TBTCSystem

• emits log events from Deposit contracts
• holds system variables
• is called when creating a new deposit to request a new keep (and pay the keep)

Tokens

• TBTCDepositToken - a standard NFT (ERC721) that tracks a deposit to the tBTC system. Can be exchanged for TBTCToken (ERC20). approveAndCall calls out to token recipient.
• FeeRebateToken - a standard NFT (ERC721) …
• TBTCToken - a standard ERC20 token and the system currency. TBTC is backed by BTC collateral. approveAndCall calls out to token recipient.

VendingMachine

• can be used to redeem TBTC for TBTCDepositToken.
• can be used to redeem TBTCDepositToken for TBTC.
• can be used to redeem BTC for TBTC via the deposit redemption flow.

Oracle

3.2 bitcoin-spv

Inheritance Structure (without usingFor)

Call Graph

Contracts

bitcoin-spv is a low-level toolkit for working with Bitcoin from other blockchains. It supplies a set of pure functions that can be used to validate almost all Bitcoin transactions and headers, as well as higher-level functions that can evaluate header chains and transaction inclusion proofs.

The bitcoin-spv project is utilized by tBTC deposits in the broader tBTC and keep system. All contracts are library contracts and the contracts with the *Delegate suffix are used to ensure the library is deployed and delegatecall’d instead of having the compiler inline the functionality. There are three main contracts, BTCUtils, CheckBitcoinSigs, and ValidateSPV.

3.3 keep-tecdsa

Inheritance Structure (without usingFor)

Call Graph

Contracts

3.4 sortition-pools

Inheritance Structure (without usingFor)

Call Graph

Contracts

3.5 keep-core

Inheritance Structure (without usingFor)

Call Graph

4 Security Specification

This section describes, from a security perspective, the expected behavior of the system under audit. It is not a substitute for documentation. The purpose of this section is to identify specific security properties that were validated by the audit team.

4.1 Oracles and Network Conditions

The project as a whole uses several sources of information for events external to Ethereum. In particular, the following oracles are used:

• Maker’s Medianizer price feed oracle is used to calculate the current price of Bitcoin relative to Ether. This value is used to calculate collateral ratios for new and existing deposits, as well as liquidation thresholds for existing collateralized deposits.
• A specialized Bitcoin difficulty relay is provided by Summa (summa-tx/relays). The relay ingests Bitcoin blockheaders and tracks the difficulty of Bitcoin proof of work over time. These values are used to ensure SPV proofs for redeemed deposits are sufficiently “confirmed.” Note that the specific behavior of the relay is out-of-scope for this audit, but is mentioned here as it is a prominent external dependency.

The following two sources of information may not fit the strict definition of an “oracle,” but are mentioned here because they each introduce an oracle-like dependency on external networks:

• The Keep Network acts as a random beacon. On request, this beacon generates random numbers and supplies them to a smart contract within the system. These random numbers are used to seed the group selection algorithm, which determines which signers become custodians of each Bitcoin deposit.
• The aforementioned signers must communicate with each other, with Bitcoin, and with the tBTC smart contracts in order to effectively manage deposits.

Reliance on external sources of data often introduces several points of failure. The efficacy of the aforementioned systems may depend on several factors:

• Bitcoin network conditions: tBTC depends on the relative reliance of the Bitcoin network during many stages of the deposit creation and redemption process. Because many stages of the deposit process are expected to happen within specific windows of time, a period of high stress in the Bitcoin network may impact the reliability of these timing windows significantly. The system may have difficulty coping with relatively longer confirmation times for deposit creation and redemption, as well as relatively higher transaction fees.
• Ethereum network conditions:
• Chain reorgs: Signing groups are only authorized to create signatures when requested via tBTC smart contracts. In the event of a chain re-org, signers may find their authorization for an already-published signature removed by the re-org. In this case, it may be possible to submit a no-longer-authorized signature for ECDSA fraud, punishing the signing group.
• Fluctuating block gas limit: Ethereum’s gas limit fluctuates over time in response to slight adjustments by miners. Among its other effects, gas limit fluctuation has a significant impact on the size of Bitcoin transactions that can be validated via SPV proof in the tBTC system contracts. Measuring this impact is crucially important during deposit creation and redemption, as even with conservative estimates, only relatively smaller Bitcoin transactions can be verified. See this issue for details.
• Fluctuating gas prices: Several components of the random beacon attempt to estimate cost (in wei) of various on-chain operations. The typical pattern used is a hardcoded gas amount multiplied by 30 GWei. The resulting amount is required to be passed in as CALLVALUE to many functions in the random beacon contracts. Although 30 GWei is a conservative estimate during periods of low network congestion, there is abundant historical evidence that gas prices often rise well above this value. Using these functions during periods of higher network congestion could result in participants receiving service at a significant discount. Conversely, it could result in participants providing service with insufficient compensation.
• Changing opcode prices: As mentioned above, many gas values are hardcoded. Should opcode prices change in a future fork, many contracts may need to undergo a costly upgrade process - or otherwise stop working correctly.
• Keep network conditions: The integrity of the Keep Network is assumed for many components of the system. Should the Keep Network cease functioning correctly, random numbers may no longer be submitted to the system contracts at expected intervals. In this case, it may become possible to game the signer group selection process and create signing groups comprised of a malicious majority of signers.

4.2 Staking Token Distribution

The keep random beacon’s purpose is in part to ensure that signers are not able to manipulate group selection. Ideally, signers are geographically distinct and do not know each other. A large factor in whether this is feasible is the distribution method for the staking token required for signer eligibility.

In order to be eligible for selection, participants are required to stake tokens. Currently, staking tokens are only available for purchase direct from Thesis, making it highly likely that staking members have few degrees of separation from each other. Although other distribution methods are planned in the future, the first iteration of this system may be more prone to signer collusion.

4.3 Signer collateral requirements

tBTC Deposits can stay active for up to 180 days, with a lot size maximum of 1 BTC. During a this active term, signers are required to lock away approximately 150% of the backing BTC’s value in ETH. Should the system experience higher-than-expected creation of Deposits, the amount of collateral available to be locked up may be depleted and deposits will not be able to be opened until others are redeemed.

4.4 Dependency - bitcoin-spv

The contract consuming the libraries functionality is expected to provide well-formed data that was verified by BTC nodes and is included in a block. The SPV verification itself involves complex security assumptions that are out of scope for the library itself. The security and trust model needs to be established with the consuming contract.

4.5 Overview - tBTC

Actors

The relevant actors are as follows:

• Funder
• TBTCDepositTokenHolder
• SigningGroup
• VendingMachine
• Other Accounts

tBTC Deposit Flow state transition incentives

This section analyzes the incentives of various actors in the system to interact and spend gas on causing a state transition for a certain Deposit.

Part of the security in the deposit flow relies on the fact that someone initiates state transitions (success path, timeout, erroneous or fraudulent behavior) as they become available. For example, if the signing group fails to form in time (3 hrs) someone is supposed to call notifySignerSetupFailure on the deposit to move it to the failed_setup end-state. On the other hand, if the signer setup succeeds, the funder is incentivized to push the deposit to the next state (awaiting_btc_funding_proof) and proceed to send BTC collateral to the address maintained by the signing group.

There are various incentives for parties in the system to invest gas in moving a deposit to another state as the transition becomes available, however, there are also incentives for not causing a transition.

Transitions

Funding: notifySignerSetupFailure

While the TDT holders main objective for the funding flow is to push the deposit to the active state as quickly as possible to redeem TBTC for TDT and earn FRT, there is no incentive for the TDT holder to call out missing timed milestones for her deposit. In one case the funder is even punished for the signing group failing to provide a valid pubKey in time leading to the funder losing the initial payment to the keep.

• TDT Holder: counter incentive. may want to avoid losing the initial payment to keep hoping the signing group returns a pubKey after the timeout passed.
• Signing Group: no incentive to spend gas.
• Others/Malicious: no incentive to spend gas.

Unless someone (an automatism) spends gas on terminating the deposit there is a good chance it may stay in this state even after the timeout passed.

Funding: retrieveSignerPubkey

There’s a strong incentive for the TDT holder to move forward being able to deposit BTC in the signer group controlled address. The signer group may provide keys to the keep contract while they have only little incentive (signer fee) to spend gas on calling retrieveSignerPubkey. The funder might end up having to call the method to proceed to the next stage.

• TDT Holder: incentive to push the deposit to be active.
• Signing Group: no incentive to spend gas. incentive for TDT is higher.
• Others/Malicious: no incentive to spend gas.

Funding: notifyFundingTimeout

funding timeout passed.

• TDT Holder: counter incentive. may want to avoid losing the initial payment to keep or potential punishment for not funding in time.
• Signing Group: honest signing group: no incentive to spend gas. Unhonest signing group: might want to front-run an attempt of TDT holder to call out provideFundingECDSAFraudProof by terminating the deposit with notifyFundingTimeout.
• Others/Malicious: no incentive to spend gas.

Funding: provideFundingECDSAFraudProof after funding timeout passed

Someone reports fraud of the signing group after the TDT holder fails to provide collateral in time. The signing group is not punished for the fraud. However, the code assumes punishment for the funder to not fund and not report in time. Additionally, if the timeout passes and the funder reports fraud but provided BTC collateral and was not able to call provideBTCFundingProof to proceed to the next stage (Note: funding proof can only be called with a delay as it requires sufficient accumulated difficulty in the header chain - x times BTC block time) the funder may lose both, the BTC collateral and can get punished for not providing a proof in time.

• TDT Holder: incentive to report fraud. However, TDT holder is punished as well as the deposit is terminated. counter-incentive to actually call out the fraud after timeout passed as the current code assumes punishment of the funder. The funder is incentivized to call notifyFundingTimeout instead which does not explicitly punish any party.
• Signing Group: counter incentive. might want to front-run an attempt of TDT holder to call out provideFundingECDSAFraudProof by terminating the deposit with notifyFundingTimeout. However, the signing group is not punished for potentially committing fraud. This might actually allow the signing group to steal BTC collateral without being punished if the TDT holder is late enough for not being able to successfully call provideBTCFundingProof before the notifyFundingTimeout.
• Others/Malicious: no incentive to spend gas.

Note: provideBTCFundingProof cannot be called immediately after entering waiting_for_btc_funding_proof as it is implicitly delayed because it requires accumulated work (x times BTC block time). In order to not lose funds, the TDT holder must ensure that provideBTCFundingProof is called before notifyFundingTimeout passes.

Note: This path does not emit logSetupFailed.

Funding: provideFundingECDSAFraudProof

The signing party committed fraud. Someone calls out the fraud in before

• TDT Holder: incentive to report fraud to be compensated with the signer bond to make up for the potentially lost funds. Ideally is able to recover the complete signer bond when being able to provide BTC funding proof, or otherwise recovers half of the funds.
• Signing Group: counter incentive. incentive to call provideBTCFundingProof in case the TDT holder actually funded the transaction to avoid provideFundingECDSAFraudProof. Furthermore, they can try to call provideECDSAFraudProof to also seize the signer bond before anyone else does.
• Others/Malicious: no incentive to spend gas. Not awarded any funds for reporting fraud.

Note: The ideal time for signers to commit fraud is right at the time the transition to provideBTCFundingProof becomes available. This way they avoid that the funder gets exclusive rights to get the signer bond awarded and they can try to report fraud themselves.

Funding: notifyFraudFundingTimeout

The signing party committed fraud. The TDT holder did not prove BTC funding in time.

• TDT Holder: counter incentive to call out the timeout. signer bond is awarded 50% to funder, 50% to signer group.
• Signing Group: incentive to call this transition before TDT holder calls provideFraudBTCFundingProof to at least recover half of the signer bond.
• Others/Malicious: no incentive to spend gas. Not awarded any funds for reporting fraud.

Note: Even though committing fraud the signer group receives half of the deposit. TDT holder is only partially compensated, losing funds if they provided BTC without being able to prove it (timeout) and potentially winning funds (depending on the keep payment) if they did not transfer BTC.

Note: Potential reward for a group of signers stealing the BTC collateral and calling out the timeout before TDT holder does is BTC collateral + 50% signer bond.

Funding: provideFraudBTCFundingProof

The signing party committed fraud. The TDT holder provided proof of transferring at minimum lotSizeSatoshis BTC to the signer group address.

• TDT Holder: incentive to get the complete signer bond awarded to cover the losses from the BTC transfer.
• Signing Group: counter incentive. Loses signer bond. favors notifyFraudFundingTimeout.
• Others/Malicious: no incentive to spend gas. Not awarded any funds for reporting fraud.

Note: The BTC funding proof requires accumulated work to pass (x times BTC block time). TDT holder must ensure to call this method before notifyFraudFundingTimeout passes or otherwise signer group could at least recover half of the signer bond.

Note: provideFraudBTCFundingProof does not verify the block timestamp of the BTC transaction. Funding can also be provided after ECDSA fraud was called to receive the full signer bond. This does not make a lot of sense as the TDT holder does not profit from this scenario unless she also colludes in the signing group and initially committed fraud.

Funding: provideBTCFundingProof

Deposit funding with collateral was proven. Deposit is in active state.

• TDT Holder: strong incentive to move to active state to redeem tBTC for TDT.
• Signing Group: incentive to get deposit to active state (signer fee). incentive to commit fraud after BTC deposit was made (backoff time for submission depending on configured accumulated difficulty) and report fraud on the active deposit to be set as liquidationInitiator which would not be possible in the funding flow.
• Others/Malicious: no incentive to spend gas.

Note: Can be called even if funding timeout passed but not called out.

Active: notifyCourtesyCall

Deposit is undercollateralized.

• TDT Holder: counter incentive to call out under-collateralization for own deposit.
• Signing Group: no incentive to spend gas.
• Others/Malicious: no incentive to spend gas other than security the system.

Note: Can be called even if the deposit term is reached.

Note: Undercollateralization can be due to oracle price slippage. (Oracle Risk)

Active: notifyDepositExpiryCourtesyCall

Deposit is reaching end-of-term.

• TDT Holder: counter incentive to call out under-collateralization for own deposit.
• Signing Group: no incentive to spend gas.
• Others/Malicious: no incentive to spend gas other than security the system.

Note: Can be called even if the deposit term is reached.

Note: This transition should be removed.

Active: exitCourtesyCall

Exit from courtesy call if deposit term is not yet reached.

• TDT Holder: incentive to set deposit to active.
• Signing Group: no incentive to spend gas.
• Others/Malicious: no incentive to spend gas.

Note: Courtesy call a can be exit in the same block if someone calls notifyDepositExpiryCourtesyCall and _d.fundedAt + TBTCConstants.getDepositTerm() == block.timestamp.

Active: notifyUndercollateralizedLiquidation, notifyCourtesyTimeout, notifySignatureTimeout, notifyRedemptionProofTimeout

Liquidate the deposit due to it being severely undercollateralized.

• TDT Holder: no incentive to spend gas.
• Signing Group: incentive to set themselves as liquidationInitiator and recover the bond at the auction. Allows one member of the signing group to purchase the bond and the signing group may receive half of the remainder after the auction to the group.
• Others/Malicious: gets rewarded for calling out undercollateralized deposits (liquidationInitiator).

Note: Calling out undercollateralized deposits can be front-run.

Note: Undercollateralization can be due to oracle price slippage. (Oracle Risk)

Active: provideECDSAFraudProof, provideSPVFraudProof

Provide proof of signer fraud for an active deposit.

• TDT Holder: incentive to report fraud to be rewarded as liquidationInitiator.
• Signing Group: counter incentive to call out fraud on themselves and incentive to report fraud to be rewarded as liquidationInitiator and recover part of the bond.
• Others/Malicious: incentive to report fraud to be rewarded as liquidationInitiator.

Redemption: provideECDSAFraudProof, provideSPVFraudProof

Provide proof of signer fraud in the redemption flow.

• Redeemer: Can be set to any address when requesting redemption. Incentive to call the transition in order to receive the full signer bond.
• Signing Group: counter incentive to call out fraud on themselves.
• Others/Malicious: no incentive to spend gas for not being rewarded.

Liquidation: purchaseSignerBondsAtAuction

Anyone can purchase the signer bond for a deposit in liquidation. The auction is settled in TBTC. The party purchasing the bond receives 90-100% of the seized bond depending on how long the auction is active already.

The longer the auction is active the more percent of the bond is awarded to the buyer. There is an incentive for the buyer to wait until the end of the auction to receive all of the signer bond. The auction can be front-run by observing that someone places a bid. The bids price is static lotSizeTbtc.

Only the leftover contract balance (which can be zero at this time if the bidder waited until the end of the auction period) the liquidationInitiator (someone calling out fraud or undercollateralized deposits or timeouts) is compensated.

• In the case of fraud the liquidationInitiator gets all the leftover contract balance

• In the case of a timeout or undercollateralized event the leftover contract balance is split between the signing group and the one calling out the event

• TDT Holder: weak incentive to purchase bonds to make sure deposit is compensated with TBTC.

• Signing Group:

  • Fraud: incentivized to bid at the latest time possible to maximize the reward and avoid compensating the party calling out fraud.
  • Abort: incentivized to bid at the latest time possible to maximize the reward and avoid compensating the party calling out the abort with more than half of the remainder after the auction value.

• Others/Malicious:

  • incentivized to bid at the latest time possible to maximize the reward and avoid compensating the party calling out fraud/abort.

• LiquidationInitiator:

• Fraud: is incentivized to maximize the reward by purchasing the bond at the earliest time possible.

• Abort: is incentivized to maximize the reward by purchasing the bond at the latest time possible.

Note: Bidding on the auction can be front-run to maximize rewards by bidding at the latest time possible.

Note: Can be front-run: observing if someone purchases the signer bond and then front-run it if lucrative.

Active: transferAndRequestRedemption, requestRedemption

Transfer TDT token ownership to a new recipient, request signer group to sign wpkhSpendSighash to initiate redemption. Minimum redemption fee is set and can only be adjusted to max. 5 times the initial redemption fee in cycles every 4 hrs with increaseRedemptionFee).

• TDT Holder: The method is supposed to be called by the current TDT holder (or VendingMachine).
• Redeemer: no incentive.
• Signing Group: no incentive.
• Others/Malicious: no incentive.

Note: Can be called after the deposit term is reached to close the deposit. However, the deposit might still fall severely undercollateralized while waiting in the redemption flow (cycling with increaseRedemptionFeefor at most 5 * 4 hours) and it will not be possible to call that out.

Redemption: provideRedemptionSignature

Signers provide the signature for the most recent wpkhSpendSighash digest.

• TDT Holder: no incentive.
• Redeemer: no incentive.
• Signing Group: provides signature to continue redemption flow and be awarded the signer fee.
• Others/Malicious: no incentive.

Note: Bails if signature for different digest ist provided.

Redemption: provideRedemptionProof

Anyone can provide proof that the BTC transaction was sent terminating the deposit.

• TDT Holder: no incentive to spend gas.
• Redeemer: no incentive to spend gas. This will reward the signers (and fee rebate to the former depositor).
• Signing Group: incentive to receive the signer fee.
• Others/Malicious: no incentive to spend gas.

Note: Does not explicitly verify tx signature. accepts previously signed transactions after increasing the fee.

Redemption: IncreaseRedemptionFee

Signers can increase the redemption fee to cover BTC transaction costs.

• TDT Holder: no incentive to spend gas.
• Redeemer: no incentive to spend gas.
• Signing Group: incentive to adjust fee.
• Others/Malicious: no incentive to spend gas.

Note: If BTC network stays congested this might always require a few cycles of provideRedemptionSignature and increaseRedemptionFee being called every 4 hrs.

Note: Can only be increased every 4 hrs to x times initial fee chosen by redeemer.

Note: Fee can be increased up to 5 * initialRedemptionFee. initialRedemptionFee can be set when requesting redemption (must be >= system minimum). There is no incentive for TDT holder or others to not increase the fee. Fee is paid from the tBTC owned by the contract.

Note: Leftover tBTC assigned to the deposit contract that is >= signerFee is sent to the rebateTokenHolder for the deposit. Values < signerFee are lost?

Note: In awaiting_withdrawal_proof a signed btc tx can be constructed to actually redeem the deposit. Assuming someone sends the transaction redeeming BTC late and increaseRedemptionFee becomes available before being able to provideRedemptionProof (delayed due to required accumulated work), someone could increase the fee after the redemption has been made. provideRedemptionProof can then afterward still be called from awaiting_withdrawal_proof.

Note: When increasing the signer fee someone could attempt to just feed in a btc transaction with the lowest signer fee as all of the signatures for the different amounts are valid.

5 Issues

Each issue has an assigned severity:

• Minor issues are subjective in nature. They are typically suggestions around best practices or readability. Code maintainers should use their own judgment as to whether to address such issues.
• Medium issues are objective in nature but are not security vulnerabilities. These should be addressed unless there is a clear reason not to.
• Major issues are security vulnerabilities that may not be directly exploitable or may require certain conditions in order to be exploited. All major issues should be addressed.
• Critical issues are directly exploitable security vulnerabilities that need to be fixed.

function recoverStake(address _operator) public {
    uint256 operatorParams = operators[_operator].packedParams;
    require(
        block.number > operatorParams.getUndelegationBlock().add(undelegationPeriod),
        "Can not recover stake before undelegation period is over."
    );

function relayEntry(bytes memory _groupSignature) public nonReentrant {
    require(isEntryInProgress(), "Entry was submitted");
    require(!hasEntryTimedOut(), "Entry timed out");

    bytes memory groupPubKey = groups.getGroupPublicKey(signingRequest.groupIndex);

    require(
        BLS.verify(
            groupPubKey,
            signingRequest.previousEntry,
            _groupSignature
        ),
        "Invalid signature"
    );

    emit RelayEntrySubmitted();

function verify(
    bytes memory publicKey,
    bytes memory message,
    bytes memory signature
) public view returns (bool) {

    AltBn128.G1Point memory _signature = AltBn128.g1Unmarshal(signature);

/**
 * @dev Unmarshals a point on G1 from bytes in an uncompressed form.
 */
function g1Unmarshal(bytes memory m) internal pure returns(G1Point memory) {
    bytes32 x;
    bytes32 y;

    /* solium-disable-next-line */
    assembly {
        x := mload(add(m, 0x20))
        y := mload(add(m, 0x40))
    }

    return G1Point(uint256(x), uint256(y));
}

// Spend no more than groupSelectionGasEstimate + 40000 gas max
// This will prevent relayEntry failure in case the service contract is compromised
signingRequest.serviceContract.call.gas(groupSelectionGasEstimate.add(40000))(
    abi.encodeWithSignature(
        "entryCreated(uint256,bytes,address)",
        signingRequest.relayRequestId,
        _groupSignature,
        msg.sender
    )
);

if (signingRequest.callbackFee > 0) {
    executeCallback(signingRequest, uint256(keccak256(_groupSignature)));
}

/// @notice Request a new keep opening.
/// @param _m Minimum number of honest keep members required to sign.
/// @param _n Number of members in the keep.
/// @return Address of a new keep.
function requestNewKeep(uint256 _m, uint256 _n, uint256 _bond)
    external
    payable
    returns (address)
{
    IBondedECDSAKeepVendor _keepVendor = IBondedECDSAKeepVendor(keepVendor);
    IBondedECDSAKeepFactory _keepFactory = IBondedECDSAKeepFactory(_keepVendor.selectFactory());
    return _keepFactory.openKeep.value(msg.value)(_n, _m, msg.sender, _bond);
}

/// @notice Set the system signer fee divisor.
/// @param _signerFeeDivisor The signer fee divisor.
function setSignerFeeDivisor(uint256 _signerFeeDivisor)
    external onlyOwner
{
    require(_signerFeeDivisor > 9, "Signer fee divisor must be greater than 9, for a signer fee that is <= 10%.");
    signerFeeDivisor = _signerFeeDivisor;
    emit SignerFeeDivisorUpdated(_signerFeeDivisor);
}

/**
 * @dev Upgrade current implementation.
 * @param _implementation Address of the new implementation contract.
 */
function upgradeTo(address _implementation)
    public
    onlyOwner
{
    address currentImplementation = implementation();
    require(_implementation != address(0), "Implementation address can't be zero.");
    require(_implementation != currentImplementation, "Implementation address must be different from the current one.");
    setImplementation(_implementation);
    emit Upgraded(_implementation);
}

/// @notice Upgrades the current vendor implementation.
/// @param _implementation Address of the new vendor implementation contract.
function upgradeTo(address _implementation) public onlyOwner {
    address currentImplementation = implementation();
    require(
        _implementation != address(0),
        "Implementation address can't be zero."
    );
    require(
        _implementation != currentImplementation,
        "Implementation address must be different from the current one."
    );
    setImplementation(_implementation);
    emit Upgraded(_implementation);
}

function registerFactory(address payable _factory) external onlyOperatorContractUpgrader {
    require(_factory != address(0), "Incorrect factory address");
    require(
        registry.isApprovedOperatorContract(_factory),
        "Factory contract is not approved"
    );
    keepFactory = _factory;
}

/**
 * @dev Function used to inform about the fact the currently ongoing
 * new relay entry generation operation timed out. As a result, the group
 * which was supposed to produce a new relay entry is immediately
 * terminated and a new group is selected to produce a new relay entry.
 * All members of the group are punished by seizing minimum stake of
 * their tokens. The submitter of the transaction is rewarded with a
 * tattletale reward which is limited to min(1, 20 / group_size) of the
 * maximum tattletale reward.
 */
function reportRelayEntryTimeout() public {
    require(hasEntryTimedOut(), "Entry did not time out");
    groups.reportRelayEntryTimeout(signingRequest.groupIndex, groupSize, minimumStake);

    // We could terminate the last active group. If that's the case,
    // do not try to execute signing again because there is no group
    // which can handle it.
    if (numberOfGroups() > 0) {
        signRelayEntry(
            signingRequest.relayRequestId,
            signingRequest.previousEntry,
            signingRequest.serviceContract,
            signingRequest.entryVerificationAndProfitFee,
            signingRequest.callbackFee
        );
    }
}

/**
 * @dev Reports unauthorized signing for the provided group. Must provide
 * a valid signature of the group address as a message. Successful signature
 * verification means the private key has been leaked and all group members
 * should be punished by seizing their tokens. The submitter of this proof is
 * rewarded with 5% of the total seized amount scaled by the reward adjustment
 * parameter and the rest 95% is burned.
 */
function reportUnauthorizedSigning(
    uint256 groupIndex,
    bytes memory signedGroupPubKey
) public {
    groups.reportUnauthorizedSigning(groupIndex, signedGroupPubKey, minimumStake);
}

function approveOperatorContract(address operatorContract) public onlyRegistryKeeper {
    operatorContracts[operatorContract] = 1;
}

function disableOperatorContract(address operatorContract) public onlyPanicButton {
    operatorContracts[operatorContract] = 2;
}

/// @notice             we poll the Keep contract to retrieve our pubkey
/// @dev                We store the pubkey as 2 bytestrings, X and Y.
/// @param  _d          deposit storage pointer
/// @return             True if successful, otherwise revert
function retrieveSignerPubkey(DepositUtils.Deposit storage _d) public {
    require(_d.inAwaitingSignerSetup(), "Not currently awaiting signer setup");

    bytes memory _publicKey = IBondedECDSAKeep(_d.keepAddress).getPublicKey();
    require(_publicKey.length == 64, "public key not set or not 64-bytes long");

function provideBTCFundingProof(
    DepositUtils.Deposit storage _d,
    bytes4 _txVersion,
    bytes memory _txInputVector,
    bytes memory _txOutputVector,
    bytes4 _txLocktime,
    uint8 _fundingOutputIndex,
    bytes memory _merkleProof,
    uint256 _txIndexInBlock,
    bytes memory _bitcoinHeaders
) public returns (bool) {

    require(_d.inAwaitingBTCFundingProof(), "Not awaiting funding");

    bytes8 _valueBytes;
    bytes memory  _utxoOutpoint;

/// @notice     Goes from courtesy call to active
/// @dev        Only callable if collateral is sufficient and the deposit is not expiring
/// @param  _d  deposit storage pointer
function exitCourtesyCall(DepositUtils.Deposit storage _d) public {
    require(_d.inCourtesyCall(), "Not currently in courtesy call");
    require(block.timestamp <= _d.fundedAt + TBTCConstants.getDepositTerm(), "Deposit is expiring");
    require(getCollateralizationPercentage(_d) >= _d.undercollateralizedThresholdPercent, "Deposit is still undercollateralized");
    _d.setActive();
    _d.logExitedCourtesyCall();
}

/// @notice     Notifies the contract that its term limit has been reached
/// @dev        This initiates a courtesy call
/// @param  _d  deposit storage pointer
function notifyDepositExpiryCourtesyCall(DepositUtils.Deposit storage _d) public {
    require(_d.inActive(), "Deposit is not active");
    require(block.timestamp >= _d.fundedAt + TBTCConstants.getDepositTerm(), "Deposit term not elapsed");
    _d.setCourtesyCall();
    _d.logCourtesyCalled();
    _d.courtesyCallInitiated = block.timestamp;
}

/// @notice             we poll the Keep contract to retrieve our pubkey
/// @dev                We store the pubkey as 2 bytestrings, X and Y.
/// @param  _d          deposit storage pointer
/// @return             True if successful, otherwise revert
function retrieveSignerPubkey(DepositUtils.Deposit storage _d) public {
    require(_d.inAwaitingSignerSetup(), "Not currently awaiting signer setup");

    bytes memory _publicKey = IBondedECDSAKeep(_d.keepAddress).getPublicKey();
    require(_publicKey.length == 64, "public key not set or not 64-bytes long");

    _d.signingGroupPubkeyX = _publicKey.slice(0, 32).toBytes32();
    _d.signingGroupPubkeyY = _publicKey.slice(32, 32).toBytes32();
    require(_d.signingGroupPubkeyY != bytes32(0) && _d.signingGroupPubkeyX != bytes32(0), "Keep returned bad pubkey");
    _d.fundingProofTimerStart = block.timestamp;

    _d.setAwaitingBTCFundingProof();
    _d.logRegisteredPubkey(
        _d.signingGroupPubkeyX,
        _d.signingGroupPubkeyY);
}

/// @notice     Anyone may notify the contract that signing group setup has timed out
/// @dev        We rely on the keep system punishes the signers in this case
/// @param  _d  deposit storage pointer
function notifySignerSetupFailure(DepositUtils.Deposit storage _d) public {
    require(_d.inAwaitingSignerSetup(), "Not awaiting setup");
    require(
        block.timestamp > _d.signingGroupRequestedAt + TBTCConstants.getSigningGroupFormationTimeout(),
        "Signing group formation timeout not yet elapsed"
    );
    _d.setFailedSetup();
    _d.logSetupFailed();

    fundingTeardown(_d);
}

/// @notice      Checks that the vin passed up is properly formatted
/// @dev         Consider a vin with a valid vout in its scriptsig
/// @param _vin  Raw bytes length-prefixed input vector
/// @return      True if it represents a validly formatted vin
function validateVin(bytes memory _vin) internal pure returns (bool) {
    uint256 _offset = 1;
    uint8 _nIns = uint8(_vin.slice(0, 1)[0]);

    // Not valid if it says there are too many or no inputs
    if (_nIns >= 0xfd || _nIns == 0) {
        return false;
    }

/// @notice          Determines the length of an output
/// @dev             5 types: WPKH, WSH, PKH, SH, and OP_RETURN
/// @param _output   The output
/// @return          The length indicated by the prefix, error if invalid length
function determineOutputLength(bytes memory _output) internal pure returns (uint256) {
    uint8 _len = uint8(_output.slice(8, 1)[0]);
    require(_len < 0xfd, "Multi-byte VarInts not supported");

    return _len + 8 + 1; // 8 byte value, 1 byte for _len itself
}

function findAndParseFundingOutput(
    DepositUtils.Deposit storage _d,
    bytes memory _txOutputVector,
    uint8 _fundingOutputIndex
) public view returns (bytes8) {

function validateAndParseFundingSPVProof(
    DepositUtils.Deposit storage _d,
    bytes4 _txVersion,
    bytes memory _txInputVector,
    bytes memory _txOutputVector,
    bytes4 _txLocktime,
    uint8 _fundingOutputIndex,
    bytes memory _merkleProof,
    uint256 _txIndexInBlock,
    bytes memory _bitcoinHeaders
) public view returns (bytes8 _valueBytes, bytes memory _utxoOutpoint){

function provideFraudBTCFundingProof(
    DepositUtils.Deposit storage _d,
    bytes4 _txVersion,
    bytes memory _txInputVector,
    bytes memory _txOutputVector,
    bytes4 _txLocktime,
    uint8 _fundingOutputIndex,
    bytes memory _merkleProof,
    uint256 _txIndexInBlock,
    bytes memory _bitcoinHeaders
) public returns (bool) {

function provideBTCFundingProof(
    DepositUtils.Deposit storage _d,
    bytes4 _txVersion,
    bytes memory _txInputVector,
    bytes memory _txOutputVector,
    bytes4 _txLocktime,
    uint8 _fundingOutputIndex,
    bytes memory _merkleProof,
    uint256 _txIndexInBlock,
    bytes memory _bitcoinHeaders
) public returns (bool) {

function provideSPVFraudProof(
    DepositUtils.Deposit storage _d,
    bytes4 _txVersion,
    bytes memory _txInputVector,
    bytes memory _txOutputVector,
    bytes4 _txLocktime,
    bytes memory _merkleProof,
    uint256 _txIndexInBlock,
    uint8 _targetInputIndex,
    bytes memory _bitcoinHeaders
) public {

/// @dev             Target is a 256 bit number encoded as a 3-byte mantissa and 1 byte exponent
/// @param _header   The header
/// @return          The target threshold
function extractTarget(bytes memory _header) internal pure returns (uint256) {
    bytes memory _m = _header.slice(72, 3);
    uint8 _e = uint8(_header[75]);
    uint256 _mantissa = bytesToUint(reverseEndianness(_m));
    uint _exponent = _e - 3;

    return _mantissa * (256 ** _exponent);
}

/// @dev             5 types: WPKH, WSH, PKH, SH, and OP_RETURN
/// @param _output   The output
/// @return          The length indicated by the prefix, error if invalid length
function determineOutputLength(bytes memory _output) internal pure returns (uint256) {
    uint8 _len = uint8(_output.slice(8, 1)[0]);
    require(_len < 0xfd, "Multi-byte VarInts not supported");

    return _len + 8 + 1; // 8 byte value, 1 byte for _len itself
}

/// @dev             Determines type by the length prefix and validates format
/// @param _output   The output
/// @return          The hash committed to by the pk_script, or null for errors
function extractHash(bytes memory _output) internal pure returns (bytes memory) {
    if (uint8(_output.slice(9, 1)[0]) == 0) {
        uint256 _len = uint8(extractOutputScriptLen(_output)[0]) - 2;
        // Check for maliciously formatted witness outputs
        if (uint8(_output.slice(10, 1)[0]) != uint8(_len)) {
            return hex"";
        }
        return _output.slice(11, _len);
    } else {
        bytes32 _tag = _output.keccak256Slice(8, 3);

function slice(bytes memory _bytes, uint _start, uint _length) internal  pure returns (bytes memory res) {
    require(_bytes.length >= (_start + _length), "Slice out of bounds");

function toUint(bytes memory _bytes, uint _start) internal  pure returns (uint256) {
    require(_bytes.length >= (_start + 32), "Uint conversion out of bounds.");

function toAddress(bytes memory _bytes, uint _start) internal  pure returns (address) {
    require(_bytes.length >= (_start + 20), "Address conversion out of bounds.");

function slice(bytes memory _bytes, uint _start, uint _length) internal  pure returns (bytes memory res) {
    require(_bytes.length >= (_start + _length), "Slice out of bounds");

function keccak256Slice(bytes memory _bytes, uint _start, uint _length) pure internal returns (bytes32 result) {
    require(_bytes.length >= (_start + _length), "Slice out of bounds");

/// @notice         Starts signer liquidation due to abort or undercollateralization
/// @dev            We first attempt to liquidate on chain, then by auction
/// @param  _d      deposit storage pointer
function startSignerAbortLiquidation(DepositUtils.Deposit storage _d) internal {
    _d.logStartedLiquidation(false);
    // Reclaim used state for gas savings
    _d.redemptionTeardown();
    _d.seizeSignerBonds();

    _d.liquidationInitiated = block.timestamp;  // Store the timestamp for auction
    _d.liquidationInitiator = msg.sender;
    _d.setFraudLiquidationInProgress();
}

/// @param _preimage        The sha256 preimage of the digest
function provideECDSAFraudProof(
    DepositUtils.Deposit storage _d,
    uint8 _v,
    bytes32 _r,
    bytes32 _s,
    bytes32 _signedDigest,
    bytes memory _preimage
) public {
    require(
        !_d.inFunding() && !_d.inFundingFailure(),
        "Use provideFundingECDSAFraudProof instead"
    );
    require(
        !_d.inSignerLiquidation(),
        "Signer liquidation already in progress"
    );
    require(!_d.inEndState(), "Contract has halted");
    require(submitSignatureFraud(_d, _v, _r, _s, _signedDigest, _preimage), "Signature is not fraud");
    startSignerFraudLiquidation(_d);
}

function provideFundingECDSAFraudProof(
    DepositUtils.Deposit storage _d,
    uint8 _v,
    bytes32 _r,
    bytes32 _s,
    bytes32 _signedDigest,
    bytes memory _preimage
) public {
    require(
        _d.inAwaitingBTCFundingProof(),
        "Signer fraud during funding flow only available while awaiting funding"
    );

    bool _isFraud = _d.submitSignatureFraud(_v, _r, _s, _signedDigest, _preimage);
    require(_isFraud, "Signature is not fraudulent");
    _d.logFraudDuringSetup();

    // If the funding timeout has elapsed, punish the funder too!
    if (block.timestamp > _d.fundingProofTimerStart + TBTCConstants.getFundingTimeout()) {
        address(0).transfer(address(this).balance);  // Burn it all down (fire emoji)
        _d.setFailedSetup();
    } else {
        /* NB: This is reuse of the variable */
        _d.fundingProofTimerStart = block.timestamp;
        _d.setFraudAwaitingBTCFundingProof();
    }
}

    uint256 contractEthBalance = address(this).balance;
    address payable initiator = _d.liquidationInitiator;

    if (initiator == address(0)){
        initiator = address(0xdead);
    }
    if (contractEthBalance > 1) {
        if (_wasFraud) {
            initiator.transfer(contractEthBalance);
        } else {
            // There will always be a liquidation initiator.
            uint256 split = contractEthBalance.div(2);
            _d.pushFundsToKeepGroup(split);
            initiator.transfer(split);
        }
    }
}

function approvedToLog(address _caller) public pure returns (bool) {
    /* TODO: auth via system */
    _caller;
    return true;
}

bytes32 resultHash = keccak256(abi.encodePacked(groupPubKey, misbehaved));

/**
 * @dev Creates a request to generate a new relay entry, which will include
 * a random number (by signing the previous entry's random number).
 * @param callbackContract Callback contract address. Callback is called once a new relay entry has been generated.
 * @param callbackMethod Callback contract method signature. String representation of your method with a single
 * uint256 input parameter i.e. "relayEntryCallback(uint256)".
 * @param callbackGas Gas required for the callback.
 * The customer needs to ensure they provide a sufficient callback gas
 * to cover the gas fee of executing the callback. Any surplus is returned
 * to the customer. If the callback gas amount turns to be not enough to
 * execute the callback, callback execution is skipped.
 * @return An uint256 representing uniquely generated relay request ID. It is also returned as part of the event.
 */
function requestRelayEntry(
    address callbackContract,
    string memory callbackMethod,
    uint256 callbackGas
) public nonReentrant payable returns (uint256) {

/**
 * @dev Executes customer specified callback for the relay entry request.
 * @param requestId Request id tracked internally by this contract.
 * @param entry The generated random number.
 * @return Address to receive callback surplus.
 */
function executeCallback(uint256 requestId, uint256 entry) public returns (address payable surplusRecipient) {
    require(
        _operatorContracts.contains(msg.sender),
        "Only authorized operator contract can call execute callback."
    );

    require(
        _callbacks[requestId].callbackContract != address(0),
        "Callback contract not found"
    );

    _callbacks[requestId].callbackContract.call(abi.encodeWithSignature(_callbacks[requestId].callbackMethod, entry));

    surplusRecipient = _callbacks[requestId].surplusRecipient;
    delete _callbacks[requestId];
}

/**
 * @dev Checks if sender is authorized.
 */
modifier onlyServiceContract() {
    require(
        serviceContracts.contains(msg.sender),
        "Caller is not an authorized contract"
    );
    _;
}

function provideRedemptionSignature(
    DepositUtils.Deposit storage _d,
    uint8 _v,
    bytes32 _r,
    bytes32 _s
) public {
    require(_d.inAwaitingWithdrawalSignature(), "Not currently awaiting a signature");

    // If we're outside of the signature window, we COULD punish signers here
    // Instead, we consider this a no-harm-no-foul situation.
    // The signers have not stolen funds. Most likely they've just inconvenienced someone

    // The signature must be valid on the pubkey
    require(
        _d.signerPubkey().checkSig(
            _d.lastRequestedDigest,
            _v, _r, _s
        ),
        "Invalid signature"
    );

// Validate `s` value for a malleability concern described in EIP-2.
// Only signatures with `s` value in the lower half of the secp256k1
// curve's order are considered valid.
require(
    uint256(_s) <=
        0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0,
    "Malleable signature - s should be in the low half of secp256k1 curve's order"
);

token.safeTransferFrom(_from, address(this), _amount);

token.transferFrom(_from, address(this), _value);

token.safeTransfer(owner, amount);

token.transfer(tattletale, tattletaleReward);

token.transferFrom(
    msg.sender,
    tokenStaking.magpieOf(members[i]),
    dividend
);

/// @notice Initializes Keep Vendor contract implementation.
/// @param registryAddress Keep registry contract linked to this contract.
function initialize(
    address registryAddress
)
    public
{
    require(!initialized(), "Contract is already initialized.");
    _initialized["BondedECDSAKeepVendorImplV1"] = true;
    registry = Registry(registryAddress);
}

function initialize(
    uint256 priceFeedEstimate,
    uint256 fluctuationMargin,
    uint256 dkgContributionMargin,
    uint256 withdrawalDelay,
    address registry
)
    public
{
    require(!initialized(), "Contract is already initialized.");
    _initialized["KeepRandomBeaconServiceImplV1"] = true;
    _priceFeedEstimate = priceFeedEstimate;
    _fluctuationMargin = fluctuationMargin;
    _dkgContributionMargin = dkgContributionMargin;
    _withdrawalDelay = withdrawalDelay;
    _pendingWithdrawal = 0;
    _previousEntry = _beaconSeed;
    _registry = registry;
    _baseCallbackGas = 18845;
}

contract DepositFactoryAuthority {

    bool internal _initialized = false;
    address internal _depositFactory;

    /// @notice Set the address of the System contract on contract initialization
    function initialize(address _factory) public {
        require(! _initialized, "Factory can only be initialized once.");

        _depositFactory = _factory;
        _initialized = true;
    }

// If subsidy pool is non-empty, distribute the value to signers but
// never distribute more than the payment for opening a keep.
uint256 signerSubsidy = subsidyPool < msg.value
    ? subsidyPool
    : msg.value;
if (signerSubsidy > 0) {
    subsidyPool -= signerSubsidy;
    keep.distributeETHToMembers.value(signerSubsidy)();
}

(bool success, ) = address(randomBeacon).call.gas(400000).value(msg.value)(
    abi.encodeWithSignature(
        "requestRelayEntry(address,string,uint256)",
        address(this),
        "setGroupSelectionSeed(uint256)",
        callbackGas
    )
);
if (!success) {
    subsidyPool += msg.value; // beacon is busy
}

/**
 * @notice Receives approval of token transfer and stakes the approved amount.
 * @dev Makes sure provided token contract is the same one linked to this contract.
 * @param _from The owner of the tokens who approved them to transfer.
 * @param _value Approved amount for the transfer and stake.
 * @param _token Token contract address.
 * @param _extraData Data for stake delegation. This byte array must have the
 * following values concatenated: Magpie address (20 bytes) where the rewards for participation
 * are sent, operator's (20 bytes) address, authorizer (20 bytes) address.
 */
function receiveApproval(address _from, uint256 _value, address _token, bytes memory _extraData) public {
    require(ERC20Burnable(_token) == token, "Token contract must be the same one linked to this contract.");
    require(_value <= token.balanceOf(_from), "Sender must have enough tokens.");
    require(_extraData.length == 60, "Stake delegation data must be provided.");

    address payable magpie = address(uint160(_extraData.toAddress(0)));
    address operator = _extraData.toAddress(20);
    require(operators[operator].owner == address(0), "Operator address is already in use.");
    address authorizer = _extraData.toAddress(40);

    // Transfer tokens to this contract.
    token.transferFrom(_from, address(this), _value);

    operators[operator] = Operator(_value, block.number, 0, _from, magpie, authorizer);
    ownerOperators[_from].push(operator);

    emit Staked(operator, _value);
}

require(block.timestamp >= _d.withdrawalRequestTime + TBTCConstants.getIncreaseFeeTimer(), "Fee increase not yet permitted");

// Check that we're incrementing the fee by exactly the redeemer's initial fee
uint256 _previousOutputValue = DepositUtils.bytes8LEToUint(_previousOutputValueBytes);
_newOutputValue = DepositUtils.bytes8LEToUint(_newOutputValueBytes);
require(_previousOutputValue.sub(_newOutputValue) == _d.initialRedemptionFee, "Not an allowed fee step");

require((_d.utxoSize().sub(_fundingOutputValue)) <= _d.initialRedemptionFee * 5, "Fee unexpectedly very high");

/// @notice Closes keep when owner decides that they no longer need it.
/// Releases bonds to the keep members. Keep can be closed only when
/// there is no signing in progress or requested signing process has timed out.
/// @dev The function can be called by the owner of the keep and only is the
/// keep has not been closed already.
function closeKeep() external onlyOwner onlyWhenActive {
    require(
        !isSigningInProgress() || hasSigningTimedOut(),
        "Requested signing has not timed out yet"
    );

    isActive = false;

    freeMembersBonds();

    emit KeepClosed();
}

/// @notice Returns bonds to the keep members.
function freeMembersBonds() internal {
    for (uint256 i = 0; i < members.length; i++) {
        keepBonding.freeBond(members[i], uint256(address(this)));
    }
}

/// @notice Releases the bond and moves the bond value to the operator's
/// unbounded value pool.
/// @dev Function requires that caller is the holder of the bond which is
/// being released.
/// @param operator Address of the bonded operator.
/// @param referenceID Reference ID of the bond.
function freeBond(address operator, uint256 referenceID) public {
    address holder = msg.sender;
    bytes32 bondID = keccak256(
        abi.encodePacked(operator, holder, referenceID)
    );

    require(lockedBonds[bondID] > 0, "Bond not found");

    uint256 amount = lockedBonds[bondID];
    lockedBonds[bondID] = 0;
    unbondedValue[operator] = amount;
}

// If the funding timeout has elapsed, punish the funder too!
if (block.timestamp > _d.fundingProofTimerStart + TBTCConstants.getFundingTimeout()) {
    address(0).transfer(address(this).balance);  // Burn it all down (fire emoji)
    _d.setFailedSetup();

function wpkhSpendSighash(
    bytes memory _outpoint,  // 36 byte UTXO id
    bytes20 _inputPKH,       // 20 byte hash160
    bytes8 _inputValue,      // 8-byte LE
    bytes8 _outputValue,     // 8-byte LE
    bytes memory _outputScript    // lenght-prefixed output script
) internal pure returns (bytes32) {
    // Fixes elements to easily make a 1-in 1-out sighash digest
    // Does not support timelocks
    bytes memory _scriptCode = abi.encodePacked(
        hex"1976a914",  // length, dup, hash160, pkh_length
        _inputPKH,
        hex"88ac");  // equal, checksig
    bytes32 _hashOutputs = abi.encodePacked(
        _outputValue,  // 8-byte LE
        _outputScript).hash256();
    bytes memory _sighashPreimage = abi.encodePacked(
        hex"01000000",  // version
        _outpoint.hash256(),  // hashPrevouts
        hex"8cb9012517c817fead650287d61bdd9c68803b6bf9c64133dcab3e65b5a50cb9",  // hashSequence(00000000)
        _outpoint,  // outpoint
        _scriptCode,  // p2wpkh script code
        _inputValue,  // value of the input in 8-byte LE
        hex"00000000",  // input nSequence
        _hashOutputs,  // hash of the single output
        hex"00000000",  // nLockTime
        hex"01000000"  // SIGHASH_ALL
    );
    return _sighashPreimage.hash256();
}

/// @notice     Closes an auction and purchases the signer bonds. Payout to buyer, funder, then signers if not fraud
/// @dev        For interface, reading auctionValue will give a past value. the current is better
/// @param  _d  deposit storage pointer
function purchaseSignerBondsAtAuction(DepositUtils.Deposit storage _d) public {
    bool _wasFraud = _d.inFraudLiquidationInProgress();
    require(_d.inSignerLiquidation(), "No active auction");

    _d.setLiquidated();
    _d.logLiquidated();

    // send the TBTC to the TDT holder. If the TDT holder is the Vending Machine, burn it to maintain the peg.
    address tdtHolder = _d.depositOwner();

    TBTCToken _tbtcToken = TBTCToken(_d.TBTCToken);

    uint256 lotSizeTbtc = _d.lotSizeTbtc();
    require(_tbtcToken.balanceOf(msg.sender) >= lotSizeTbtc, "Not enough TBTC to cover outstanding debt");

    if(tdtHolder == _d.VendingMachine){
        _tbtcToken.burnFrom(msg.sender, lotSizeTbtc);  // burn minimal amount to cover size
    }
    else{
        _tbtcToken.transferFrom(msg.sender, tdtHolder, lotSizeTbtc);
    }

    // Distribute funds to auction buyer
    uint256 _valueToDistribute = _d.auctionValue();
    msg.sender.transfer(_valueToDistribute);

    // Send any TBTC left to the Fee Rebate Token holder
    _d.distributeFeeRebate();

    // For fraud, pay remainder to the liquidation initiator.
    // For non-fraud, split 50-50 between initiator and signers. if the transfer amount is 1,
    // division will yield a 0 value which causes a revert; instead, 
    // we simply ignore such a tiny amount and leave some wei dust in escrow
    uint256 contractEthBalance = address(this).balance;
    address payable initiator = _d.liquidationInitiator;

    if (initiator == address(0)){
        initiator = address(0xdead);
    }
    if (contractEthBalance > 1) {
        if (_wasFraud) {
            initiator.transfer(contractEthBalance);
        } else {
            // There will always be a liquidation initiator.
            uint256 split = contractEthBalance.div(2);
            _d.pushFundsToKeepGroup(split);
            initiator.transfer(split);
        }
    }
}