Skip to content

Commit

Permalink
Merge #939: fix: reject message (de)serialization
Browse files Browse the repository at this point in the history
548725c test: reject message (de)serialization (0xb10c)
fc572ab fix: use var_str in 'reject' msgs (0xb10c)

Pull request description:

  [BIP-61 defines `response-to-msg`][bip61] (`Reject::message` in rust-bitcoin; the message that triggered the reject) to be a `var_str`. However, by using the `CommandString` it was (de)serialized as 12 byte string. A test is added that de- and serializes two reject messages received from an older Bitcoin Core peer.

  Reject message sending has been removed from Bitcoin Core, I'm still receiving them from older peers from time to time.

  [bip61]: https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki#common-payload

  gh-ref: #323

ACKs for top commit:
  apoelstra:
    ACK 548725c

Tree-SHA512: e5cbf215a471f113b4dd7f7fada162686fc6e8c7b1e2e9e641667208a36d3db610e57e8b549756ffe597656fee5444fe95466f1b88f45366595766f7c4640eea
  • Loading branch information
apoelstra committed Apr 20, 2022
2 parents 1e58208 + 548725c commit 954b8a9
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/network/message.rs
Expand Up @@ -472,7 +472,7 @@ mod test {
NetworkMessage::GetCFCheckpt(GetCFCheckpt{filter_type: 17, stop_hash: hash([25u8; 32]).into()}),
NetworkMessage::CFCheckpt(CFCheckpt{filter_type: 27, stop_hash: hash([77u8; 32]).into(), filter_headers: vec![hash([3u8; 32]).into(), hash([99u8; 32]).into()]}),
NetworkMessage::Alert(vec![45,66,3,2,6,8,9,12,3,130]),
NetworkMessage::Reject(Reject{message: CommandString::try_from("Test reject").unwrap(), ccode: RejectReason::Duplicate, reason: "Cause".into(), hash: hash([255u8; 32])}),
NetworkMessage::Reject(Reject{message: "Test reject".into(), ccode: RejectReason::Duplicate, reason: "Cause".into(), hash: hash([255u8; 32])}),
NetworkMessage::FeeFilter(1000),
NetworkMessage::WtxidRelay,
NetworkMessage::AddrV2(vec![AddrV2Message{ addr: AddrV2::Ipv4(Ipv4Addr::new(127, 0, 0, 1)), port: 0, services: ServiceFlags::NONE, time: 0 }]),
Expand Down
39 changes: 37 additions & 2 deletions src/network/message_network.rs
Expand Up @@ -26,7 +26,6 @@ use network::address::Address;
use network::constants::{self, ServiceFlags};
use consensus::{Encodable, Decodable, ReadExt};
use consensus::encode;
use network::message::CommandString;
use hashes::sha256d;

/// Some simple messages
Expand Down Expand Up @@ -133,7 +132,7 @@ impl Decodable for RejectReason {
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct Reject {
/// message type rejected
pub message: CommandString,
pub message: Cow<'static, str>,
/// reason of rejection as code
pub ccode: RejectReason,
/// reason of rejectection
Expand All @@ -147,8 +146,11 @@ impl_consensus_encoding!(Reject, message, ccode, reason, hash);
#[cfg(test)]
mod tests {
use super::VersionMessage;
use super::Reject;
use super::RejectReason;

use hashes::hex::FromHex;
use hashes::sha256d::Hash;
use network::constants::ServiceFlags;

use consensus::encode::{deserialize, serialize};
Expand All @@ -172,4 +174,37 @@ mod tests {

assert_eq!(serialize(&real_decode), from_sat);
}

#[test]
fn reject_message_test() {
let reject_tx_conflict = Vec::from_hex("027478121474786e2d6d656d706f6f6c2d636f6e666c69637405df54d3860b3c41806a3546ab48279300affacf4b88591b229141dcf2f47004").unwrap();
let reject_tx_nonfinal = Vec::from_hex("02747840096e6f6e2d66696e616c259bbe6c83db8bbdfca7ca303b19413dc245d9f2371b344ede5f8b1339a5460b").unwrap();

let decode_result_conflict: Result<Reject, _> = deserialize(&reject_tx_conflict);
let decode_result_nonfinal: Result<Reject, _> = deserialize(&reject_tx_nonfinal);

assert!(decode_result_conflict.is_ok());
assert!(decode_result_nonfinal.is_ok());

let conflict = decode_result_conflict.unwrap();
assert_eq!("tx", conflict.message);
assert_eq!(RejectReason::Duplicate, conflict.ccode);
assert_eq!("txn-mempool-conflict", conflict.reason);
assert_eq!(
Hash::from_hex("0470f4f2dc4191221b59884bcffaaf00932748ab46356a80413c0b86d354df05").unwrap(),
conflict.hash
);

let nonfinal = decode_result_nonfinal.unwrap();
assert_eq!("tx", nonfinal.message);
assert_eq!(RejectReason::NonStandard, nonfinal.ccode);
assert_eq!("non-final", nonfinal.reason);
assert_eq!(
Hash::from_hex("0b46a539138b5fde4e341b37f2d945c23d41193b30caa7fcbd8bdb836cbe9b25").unwrap(),
nonfinal.hash
);

assert_eq!(serialize(&conflict), reject_tx_conflict);
assert_eq!(serialize(&nonfinal), reject_tx_nonfinal);
}
}

0 comments on commit 954b8a9

Please sign in to comment.