Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spy detector in Peer-observer #45

Closed
wants to merge 61 commits into from

Conversation

i-am-yuvi
Copy link
Collaborator

@i-am-yuvi i-am-yuvi commented Aug 26, 2024

Fixes #43

This creates a tool, Spy-Detector that:

  • prints stats of every peer(inv, tx, getdata) at every 10 secs
  • for connection being closed their stats are printed and removed from shared state

Copy link
Owner

@0xB10C 0xB10C left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INV and GetData messages can contain e.g. blocks too and not only transactions. You currently use the msg type string from the Metadata, but it's better to match on the message payload. You can filter out block INV and GetData messages there.

Cargo.toml Show resolved Hide resolved
@i-am-yuvi
Copy link
Collaborator Author

INV and GetData messages can contain e.g. blocks too and not only transactions. You currently use the msg type string from the Metadata, but it's better to match on the message payload. You can filter out block INV and GetData messages there.

Agree!!

@i-am-yuvi
Copy link
Collaborator Author

INV and GetData messages can contain e.g. blocks too and not only transactions. You currently use the msg type string from the Metadata, but it's better to match on the message payload. You can filter out block INV and GetData messages there.

So basically INVs can contain blocks too, my question is:

  • spy nodes can relay block inv but not transaction inv?
  • can INV messages contain both Block and Tx at the same time?

@0xB10C
Copy link
Owner

0xB10C commented Sep 1, 2024

So basically INVs can contain blocks too, my question is:

See https://docs.rs/bitcoin/latest/bitcoin/p2p/message_blockdata/enum.Inventory.html

We want to handle Inventory::Transaction and Inventory::WTx (protobuf definitions here

bytes transaction = 1;
bytes block = 2;
// MSG_WTX (0x00000005) as defined in BIP 339 (wtxid based tx-relay)
bytes wtx = 3;
) for INVs. We might need to handle Inventory::WitnessTransaction (
bytes witness_transaction = 4;
) too for GET_DATA.

See also https://github.com/bitcoin/bitcoin/blob/5abb9b1af49be9024f95fa2f777285c531785d85/src/protocol.h#L473-L490 and the comments there.

  • spy nodes can relay block inv but not transaction inv?

I think we can ignore block invs for now. Here, we are only interested in TX / WTX invs.

  • can INV messages contain both Block and Tx at the same time?

Yes, that's possible, but not too common. Best would be to iterate over an INV message and increment the counter for each transaction/witness_transaction in the INV message.

@i-am-yuvi
Copy link
Collaborator Author

So basically INVs can contain blocks too, my question is:

See https://docs.rs/bitcoin/latest/bitcoin/p2p/message_blockdata/enum.Inventory.html

We want to handle Inventory::Transaction and Inventory::WTx (protobuf definitions here

bytes transaction = 1;
bytes block = 2;
// MSG_WTX (0x00000005) as defined in BIP 339 (wtxid based tx-relay)
bytes wtx = 3;

) for INVs. We might need to handle Inventory::WitnessTransaction (

bytes witness_transaction = 4;

) too for GET_DATA.
See also https://github.com/bitcoin/bitcoin/blob/5abb9b1af49be9024f95fa2f777285c531785d85/src/protocol.h#L473-L490 and the comments there.

  • spy nodes can relay block inv but not transaction inv?

I think we can ignore block invs for now. Here, we are only interested in TX / WTX invs.

  • can INV messages contain both Block and Tx at the same time?

Yes, that's possible, but not too common. Best would be to iterate over an INV message and increment the counter for each transaction/witness_transaction in the INV message.

great!! Thanks for sharing!

@i-am-yuvi i-am-yuvi self-assigned this Sep 2, 2024
@i-am-yuvi
Copy link
Collaborator Author

I think we can define primitive for Getdata too! @0xB10C ?

@0xB10C
Copy link
Owner

0xB10C commented Sep 2, 2024

We already have one!

@i-am-yuvi
Copy link
Collaborator Author

We already have one!

Okay got it!!

@0xB10C
Copy link
Owner

0xB10C commented Oct 30, 2024

Hey, are you still working on this?

@i-am-yuvi
Copy link
Collaborator Author

Hey, are you still working on this?

Yes, I am working!!

@i-am-yuvi i-am-yuvi marked this pull request as ready for review November 3, 2024 11:07
@i-am-yuvi
Copy link
Collaborator Author

i-am-yuvi commented Nov 3, 2024

Need to resolve conflicts and squash commits!!
I will do that soon!

@i-am-yuvi i-am-yuvi requested a review from 0xB10C November 3, 2024 11:10
@i-am-yuvi i-am-yuvi changed the title [Do Not Merger] Spy detector in Peer-observer Spy detector in Peer-observer Nov 4, 2024
@i-am-yuvi
Copy link
Collaborator Author

Continued in #57 , I'm closing this for now!

@i-am-yuvi i-am-yuvi closed this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically Detecting Spy Peers
2 participants