LayerTwo-Labs / bip300301_enforcer

CUSF software enforcing BIP300 and BIP301 rules.
1 stars 4 forks source link

multi: implement create_bmm_critical_data_transaction #38

Closed torkelrogstad closed 1 week ago

torkelrogstad commented 2 weeks ago

Closes #43

TODO:

The code here has been used to construct seemingly valid BMM requests, for example here: https://mempool.drivechain.live/tx/48304f36b955f45901af77d2aa8f61893f7687ba3332601ad326da664bad1575

To try this for yourselves, you'll have to apply this patch in order to bypass the active sidechain + correct main hash checks:

diff --git a/src/server.rs b/src/server.rs
index 1d23b11..93f7802 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -574,6 +574,7 @@ impl WalletService for Arc<crate::wallet::Wallet> {
                 )
             })?;

+        /*
         match self.is_sidechain_active(sidechain_number).await {
             Ok(false) => {
                 return Err(tonic::Status::failed_precondition(
@@ -582,7 +583,7 @@ impl WalletService for Arc<crate::wallet::Wallet> {
             }
             Ok(true) => (),
             Err(err) => return Err(tonic::Status::from_error(err.into())),
-        }
+        } */

         // This is also called H*
         let critical_hash = critical_hash
@@ -603,7 +604,7 @@ impl WalletService for Arc<crate::wallet::Wallet> {

         // If the mainchain tip has progressed beyond this, the request is already
         // expired.
-        if mainchain_tip != bdk_block_hash_to_bitcoin_block_hash(prev_bytes) {
+        if false && mainchain_tip != bdk_block_hash_to_bitcoin_block_hash(prev_bytes) {
             let message = format!(
                 "invalid prev_bytes {}: expected {}",
                 prev_bytes, mainchain_tip

You can then try the endpoint:

$ buf curl --protocol grpc --http2-prior-knowledge --emit-defaults -d '{"value_sats": 10000, "height": 123, "sidechain_id": 1, "critical_hash": {"hex": "1200007c0ca1efd8d128fedf50c73f395b0cceb0ffa823edbd971b4afd913021"}, "prev_bytes": {"hex": "000000e597b4ff2b7b81bcb2b0199b65471b9ece52b9688f18a8a7ed8e275eb1"}}' http://localhost:50051/cusf.mainchain.v1.WalletService/CreateBmmCriticalDataT
ransaction

{
  "txid": {
    "hex": "516a86d7e375cd3295288eaed9b691734e5a218e7e02c99fff6e8196977b84b4"
  }
}

Note that in this case the hex printed to the console is not the TXID of the broadcasted TX! This is because the txid field of the response here is of type ConsensusHex, and not ReverseHex. The actual TXID is b4847b9796816eff9fc9027e8e215a4e7391b6d9ae8e289532cd75e3d7866a51.

Ash-L2L commented 2 weeks ago

The multiple versions of bitcoin is annoying, and something that happens quite frequently as the bitcoin crate often makes breaking API changes. Trying to maintain a single version is futile. The best approach is to consistently use a recent version, and to use older versions compatible with dependencies as little as possible. Most crates using the bitcoin crate re-export it so that this is easy to do.

If it were reasonable to do so, I'd lint for this

use other_crate::bitcoin::{...};

except when aliased or anonymous, ie.

use other_crate::bitcoin::{
    self as other_crate_bitcoin, TxOut as OtherCrateTxOut, consensus::Encodable as _,
};

There are some exceptions though, eg. the wallet code needs to be compatible with BDK, so using bdk::bitcoin at the module scope is fine.

Since this occurs often, perhaps we should publish a crate that facilitates easy conversion between types from different versions of bitcoin?

torkelrogstad commented 2 weeks ago

Since this occurs often, perhaps we should publish a crate that facilitates easy conversion between types from different versions of bitcoin?

Wouldn't a new create just make the problem even worse by introducing yet another version? Or perhaps it's possible to implement this in a version-agnostic way. If so, that'd be really nice.

nchashch commented 2 weeks ago

This should close #43

nchashch commented 2 weeks ago

We're using two different version of rust-bitcoin throughout the project. One from bitcoin, and one re-exported from bdk. This leads to a bit wrangling back and forth to get the compiler to be happy. Would be very nice to converge on only one of these versions.

This should be fixed in a separate PR.

I opened an issue for this: #45

nchashch commented 2 weeks ago

This RPC should insert a row into the bmm_requests table that was added in #41