diff --git a/Cargo.lock b/Cargo.lock index e26e8a8f284..8d86e78ebbc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1659,7 +1659,7 @@ dependencies = [ [[package]] name = "dash-network" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "cbindgen 0.29.4", "grovedb-bincode", @@ -1670,7 +1670,7 @@ dependencies = [ [[package]] name = "dash-network-seeds" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "dash-network", ] @@ -1767,7 +1767,7 @@ dependencies = [ [[package]] name = "dash-spv" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "async-trait", "chrono", @@ -1796,7 +1796,7 @@ dependencies = [ [[package]] name = "dashcore" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "anyhow", "base64-compat", @@ -1822,12 +1822,12 @@ dependencies = [ [[package]] name = "dashcore-private" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" [[package]] name = "dashcore-rpc" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "dashcore-rpc-json", "hex", @@ -1840,7 +1840,7 @@ dependencies = [ [[package]] name = "dashcore-rpc-json" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "dashcore", "grovedb-bincode", @@ -1855,7 +1855,7 @@ dependencies = [ [[package]] name = "dashcore_hashes" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "dashcore-private", "grovedb-bincode", @@ -2934,7 +2934,7 @@ dependencies = [ [[package]] name = "git-state" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" [[package]] name = "glob" @@ -4166,7 +4166,7 @@ dependencies = [ [[package]] name = "key-wallet" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "aes", "async-trait", @@ -4195,7 +4195,7 @@ dependencies = [ [[package]] name = "key-wallet-ffi" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "cbindgen 0.29.4", "dash-network", @@ -4211,7 +4211,7 @@ dependencies = [ [[package]] name = "key-wallet-manager" version = "0.45.0" -source = "git+https://github.com/dashpay/rust-dashcore?rev=379f0574ecc676e4c328182eb094009fda204ed6#379f0574ecc676e4c328182eb094009fda204ed6" +source = "git+https://github.com/dashpay/rust-dashcore?rev=edc4d4305d7857547647f84c4974dab6ed363e4f#edc4d4305d7857547647f84c4974dab6ed363e4f" dependencies = [ "async-trait", "dashcore", diff --git a/Cargo.toml b/Cargo.toml index a354103b387..8d3023e6242 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,14 +65,14 @@ grovedb-storage = { git = "https://github.com/dashpay/grovedb", rev = "fc7028d83 grovedb-version = { git = "https://github.com/dashpay/grovedb", rev = "fc7028d83114d86f88222e7ff09fc47ae75360f0" } grovedb-epoch-based-storage-flags = { git = "https://github.com/dashpay/grovedb", rev = "fc7028d83114d86f88222e7ff09fc47ae75360f0" } grovedb-commitment-tree = { git = "https://github.com/dashpay/grovedb", rev = "fc7028d83114d86f88222e7ff09fc47ae75360f0" } -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } -dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "379f0574ecc676e4c328182eb094009fda204ed6" } +dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } +dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "edc4d4305d7857547647f84c4974dab6ed363e4f" } tokio-metrics = "0.5" # Size-tuned profile for the iOS `rs-unified-sdk-ffi` staticlib, which diff --git a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs index 3873ae8aecb..c77951da2dd 100644 --- a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs +++ b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs @@ -6,9 +6,8 @@ use crate::serialization::JsonConvertible; use crate::serialization::ValueConvertible; use std::convert::{TryFrom, TryInto}; -use dashcore::consensus::encode::{self, VarInt}; -use dashcore::consensus::{deserialize, deserialize_partial, Decodable, Encodable}; -use dashcore::transaction::special_transaction::{TransactionPayload, TransactionType}; +use dashcore::consensus::{deserialize, Encodable}; +use dashcore::transaction::special_transaction::TransactionPayload; use dashcore::{InstantLock, OutPoint, Transaction, TxIn, TxOut}; use platform_value::{BinaryData, Value}; @@ -207,40 +206,19 @@ pub struct RawInstantLockProof { output_index: u32, } -/// Decodes an instant proof's transaction. An asset lock decodes as it always has on every -/// node: its payload is read after its length prefix, whatever length the prefix announces, -/// and must end the bytes. Neither the InstantSend lock nor the transition's signature covers -/// the prefix, so a node that read the payload through it would refuse proofs the other nodes -/// accept and pay for. Any other transaction decodes as usual. -fn decode_asset_lock_transaction(bytes: &[u8]) -> Result { - let [v0, v1, t0, t1, ..] = *bytes else { - return deserialize(bytes); - }; - // A transaction carries a payload only from version 3, read as a signed 16-bit value. - if i16::from_le_bytes([v0, v1]) < 3 - || u16::from_le_bytes([t0, t1]) != TransactionType::AssetLock.to_u16() - { - return deserialize(bytes); - } - // Up to the payload, an asset lock decodes as a classic transaction does, with the - // same segwit rule, and a classic transaction has no payload section. - let mut body = bytes.to_vec(); - body[2..4].fill(0); - let (mut transaction, payload_start) = deserialize_partial::(&body)?; - let mut payload = &bytes[payload_start..]; - VarInt::consensus_decode(&mut payload)?; - transaction.special_transaction_payload = Some(TransactionPayload::AssetLockPayloadType( - deserialize(payload)?, - )); - Ok(transaction) -} - impl TryFrom for InstantAssetLockProof { type Error = ProtocolError; fn try_from(raw_instant_lock: RawInstantLockProof) -> Result { - let transaction = decode_asset_lock_transaction(raw_instant_lock.transaction.as_slice()) - .map_err(|e| ProtocolError::DecodingError(e.to_string()))?; + // An asset lock decodes as it always has on every node: its payload is read after its + // length prefix, a minimally encoded CompactSize whatever length it announces, and must + // end the bytes. Neither the InstantSend lock nor the transition's signature covers the + // prefix, so a node that read the payload through it would refuse proofs the other nodes + // accept and pay for. + let transaction = Transaction::deserialize_with_legacy_asset_lock_framing( + raw_instant_lock.transaction.as_slice(), + ) + .map_err(|e| ProtocolError::DecodingError(e.to_string()))?; let instant_lock = deserialize(raw_instant_lock.instant_lock.as_slice()) .map_err(|e| ProtocolError::DecodingError(e.to_string()))?; @@ -279,6 +257,11 @@ impl TryFrom<&InstantAssetLockProof> for RawInstantLockProof { mod tests { use super::*; use crate::tests::fixtures::raw_instant_asset_lock_proof_fixture; + use dashcore::consensus::encode::{self, VarInt}; + use dashcore::consensus::{deserialize_partial, Decodable}; + use dashcore::transaction::special_transaction::TransactionType; + use rand::rngs::StdRng; + use rand::{Rng, SeedableRng}; // --------------------------------------------------------------- // Default @@ -596,7 +579,15 @@ mod tests { #[test] fn test_raw_instant_lock_proof_reads_an_asset_lock_payload_whatever_its_length_prefix() { let canonical = hex::decode(CORE_ASSET_LOCK).unwrap(); - for prefix in [&[0x23][..], &[0x25], &[0x00], &[0xfc], &[0xfd, 0x00, 0x01]] { + for prefix in [ + &[0x23][..], + &[0x25], + &[0x00], + &[0xfc], + &[0xfd, 0x00, 0x01], + &[0xfe, 0x00, 0x00, 0x01, 0x00], + &[0xff, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00], + ] { let proof = decode_core_asset_lock(core_asset_lock_with_payload_length_prefix(prefix)) .unwrap_or_else(|e| panic!("prefix {prefix:02x?}: {e}")); assert_eq!( @@ -642,6 +633,136 @@ mod tests { assert!(decode_core_asset_lock(transaction_bytes).is_err(), "{case}"); } } + + /// A byte-level decoder of proof transactions that nodes have run: an asset lock is decoded + /// as a classic transaction up to its payload, then its length prefix is read and ignored, + /// and the payload must end the bytes. Anything else goes to `deserialize`. + /// + /// It decodes inputs, outputs and payloads with the pinned rust-dashcore, as rust-dashcore's + /// own decoder does, so comparing the two cannot see a change to those shared decoders; the + /// fixed txids and identifiers in the tests above guard them. + fn byte_level_asset_lock_decoder(bytes: &[u8]) -> Result { + let [v0, v1, t0, t1, ..] = *bytes else { + return deserialize(bytes); + }; + if i16::from_le_bytes([v0, v1]) < 3 + || u16::from_le_bytes([t0, t1]) != TransactionType::AssetLock.to_u16() + { + return deserialize(bytes); + } + let mut body = bytes.to_vec(); + body[2..4].fill(0); + let (mut transaction, payload_start) = deserialize_partial::(&body)?; + let mut payload = &bytes[payload_start..]; + VarInt::consensus_decode(&mut payload)?; + transaction.special_transaction_payload = Some(TransactionPayload::AssetLockPayloadType( + deserialize(payload)?, + )); + Ok(transaction) + } + + /// Nodes that ran the byte-level decoder and nodes that run rust-dashcore's must agree on + /// every proof transaction: whether it decodes, to what, and with which error, or they + /// would disagree on transitions that are paid for. Seeded mutations of a real asset lock, + /// of its witness form and of its form without inputs, many with a rewritten payload + /// length prefix, cover both the decoding and the refusal paths. + #[test] + fn test_raw_instant_lock_proof_decodes_every_transaction_as_the_byte_level_decoder_did() { + let canonical = hex::decode(CORE_ASSET_LOCK).unwrap(); + // Version and type, the input count and its one input, the two outputs and the lock + // time, then the payload length prefix and the 36-byte payload. + let (header, rest) = canonical.split_at(4); + let (inputs, rest) = rest.split_at(148); + let (outputs, rest) = rest.split_at(rest.len() - 41); + let (lock_time, payload) = rest.split_at(4); + assert_eq!(payload[0], 36); + let witness_form = [ + header, + &[0, 1], + inputs, + outputs, + &[1, 1, 0xaa], + lock_time, + payload, + ] + .concat(); + let without_inputs = [header, &[0, 1, 0], outputs, lock_time, payload].concat(); + // Both forms decode, so their mutations reach the decoding path, not only refusals. + assert_eq!( + Transaction::deserialize_with_legacy_asset_lock_framing(&witness_form) + .expect("the witness form decodes") + .txid(), + deserialize::(&canonical).unwrap().txid() + ); + Transaction::deserialize_with_legacy_asset_lock_framing(&without_inputs) + .expect("the asset lock without inputs decodes"); + let seeds = [canonical.clone(), witness_form, without_inputs]; + + let mut rng = StdRng::seed_from_u64(0x5ea1_ed00); + let mut decoded_from_other_bytes = 0; + // Per prefix width (1, 3, 5, 9 bytes): rewritten prefixes that still decoded. + let mut decoded_per_prefix_width = [0; 4]; + for _ in 0..100_000 { + let mut bytes = seeds[rng.gen_range(0..seeds.len())].clone(); + let mut prefix_width = None; + if rng.gen_bool(0.5) { + let prefix_at = bytes.len() - 37; + let width = rng.gen_range(0..4); + prefix_width = Some(width); + let prefix: Vec = match width { + 0 => vec![rng.gen()], + 1 => [&[0xfd][..], &rng.gen::().to_le_bytes()].concat(), + 2 => [&[0xfe][..], &rng.gen::().to_le_bytes()].concat(), + _ => [&[0xff][..], &rng.gen::().to_le_bytes()].concat(), + }; + bytes.splice(prefix_at..=prefix_at, prefix); + } + for _ in 0..rng.gen_range(0..3) { + let at = rng.gen_range(0..=bytes.len()); + match rng.gen_range(0..4) { + 0 if at < bytes.len() => bytes[at] = rng.gen(), + 1 if at < bytes.len() => { + bytes.remove(at); + } + 2 => bytes.truncate(at), + _ => bytes.insert(at, rng.gen()), + } + } + + let decoded = Transaction::deserialize_with_legacy_asset_lock_framing(&bytes); + match (&decoded, byte_level_asset_lock_decoder(&bytes)) { + (Ok(transaction), Ok(expected)) => { + assert_eq!(transaction, &expected, "{}", hex::encode(&bytes)); + if dashcore::consensus::serialize(transaction) != bytes { + decoded_from_other_bytes += 1; + } + if let Some(width) = prefix_width { + decoded_per_prefix_width[width] += 1; + } + } + (Err(error), Err(expected)) => { + assert_eq!( + error.to_string(), + expected.to_string(), + "{}", + hex::encode(&bytes) + ) + } + (_, expected) => panic!( + "{}: {decoded:?}, but the byte-level decoder gives {expected:?}", + hex::encode(&bytes) + ), + } + } + assert!( + decoded_from_other_bytes > 1_000, + "only {decoded_from_other_bytes} non-canonical transactions decoded" + ); + assert!( + decoded_per_prefix_width.iter().all(|&count| count > 0), + "decoded per prefix width: {decoded_per_prefix_width:?}" + ); + } } #[cfg(all( diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create/mod.rs index 751c6271890..766a40c02f5 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create/mod.rs @@ -497,21 +497,28 @@ mod tests { /// the same identity, at the same fee, with the same balance. #[tokio::test] async fn should_create_the_same_identity_from_a_proof_with_another_payload_length_prefix() { - let original = create_identity_with_proof_payload_length_prefix_shifted_by(0).await; - for shift in [1, -1] { + let original = + create_identity_with_proof_payload_length_prefix(|length| vec![length]).await; + let prefixes: [(&str, fn(u8) -> Vec); 4] = [ + ("one more", |length| vec![length + 1]), + ("one less", |length| vec![length - 1]), + ("three bytes wide", |_| vec![0xfd, 0x00, 0x01]), + ("five bytes wide", |_| vec![0xfe, 0x00, 0x00, 0x01, 0x00]), + ]; + for (case, prefix) in prefixes { assert_eq!( - create_identity_with_proof_payload_length_prefix_shifted_by(shift).await, + create_identity_with_proof_payload_length_prefix(prefix).await, original, - "prefix shifted by {shift}" + "prefix {case}" ); } } /// Executes, at the latest protocol version, an IdentityCreate whose serialized instant - /// proof has its asset-lock payload length prefix shifted by `shift`, and returns the - /// processing fee and the new identity's balance. - async fn create_identity_with_proof_payload_length_prefix_shifted_by( - shift: i16, + /// proof has its 1-byte asset-lock payload length prefix replaced by `prefix(length)`, and + /// returns the processing fee and the new identity's balance. + async fn create_identity_with_proof_payload_length_prefix( + prefix: fn(u8) -> Vec, ) -> (dpp::fee::Credits, dpp::fee::Credits) { use dpp::dashcore::consensus::serialize; use dpp::prelude::AssetLockProof; @@ -611,8 +618,18 @@ mod tests { .windows(proof_transaction.len()) .position(|bytes| bytes == proof_transaction.as_slice()) .expect("expected the proof transaction in the serialized transition"); - let prefix = &mut identity_create_serialized_transition[transaction_at + prefix_at]; - *prefix = u8::try_from(i16::from(*prefix) + shift).expect("1-byte prefix"); + let new_prefix = prefix(proof_transaction[prefix_at]); + // The proof's transaction bytes follow their bincode length, a single byte below 251. + let length = &mut identity_create_serialized_transition[transaction_at - 1]; + assert_eq!(usize::from(*length), proof_transaction.len()); + *length = u8::try_from(proof_transaction.len() - 1 + new_prefix.len()) + .ok() + .filter(|length| *length < 251) + .expect("a 1-byte bincode length"); + identity_create_serialized_transition.splice( + transaction_at + prefix_at..=transaction_at + prefix_at, + new_prefix.iter().copied(), + ); let transaction = platform.drive.grove.start_transaction(); @@ -632,7 +649,7 @@ mod tests { assert_eq!( processing_result.valid_count(), 1, - "prefix shifted by {shift}" + "prefix {new_prefix:02x?}" ); platform