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

chore[arbitrary]: make arbitrary and proptest an optional dependency #316

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

skaunov
Copy link

@skaunov skaunov commented Jan 8, 2025

Resolve #188

It's WIP yet - though I'd be happy if you advice on the linting issues I had to add to commit.

Also I'm looking into property testing and wonder if there's another task to get hands on if you'd suggest one.

@skaunov skaunov changed the title basic approach Resolve #188 Jan 8, 2025
@skaunov skaunov marked this pull request as draft January 8, 2025 15:25
@Sword-Smith
Copy link
Member

I have two ideas:

  • You can ensure that the same peer never requests the same block more than twice. And if they do, they can be punished for that. peer_loop would be where that functionality would live.
  • You can work on style: Block::is_valid_internal #293, change its return type to result type, and start adding negative tests of all failure modes.

@skaunov skaunov marked this pull request as ready for review January 9, 2025 18:36
@skaunov skaunov changed the title Resolve #188 chore[arbitrary]: make arbitrary and proptest an optional dependency Jan 9, 2025
@skaunov
Copy link
Author

skaunov commented Jan 9, 2025

Thanks, it's great to have a choice! I'll take a look. Meanwhile this one can be merged I guess.

@skaunov
Copy link
Author

skaunov commented Jan 10, 2025

I started to look into the tests of peer_loop -- will take some time as I didn't even start the proptest book from any end. What is considered to request twice? A time frame, or a consecutive request?

  • You can ensure that the same peer never requests the same block more than twice. And if they do, they can be punished for that. peer_loop would be where that functionality would live.

@Sword-Smith
Copy link
Member

Sword-Smith commented Jan 10, 2025

I started to look into the tests of peer_loop -- will take some time as I didn't even start the proptest from any end. What is considered to request twice? A time frame, or a consecutive request?

  • You can ensure that the same peer never requests the same block more than twice. And if they do, they can be punished for that. peer_loop would be where that functionality would live.

There are a few ways that peers can request blocks. E.g. by PeerMessage::BlockRequestByHeight and PeerMessage::BlockRequestByHash. If successful, this will result in the sharing of a block through a call like.

peer.send(PeerMessage::Block(Box::new(b.try_into().unwrap())))
    .await?;

Every time a block is shared with a peer, it could be recorded in a field of type HashMap<Digest, usize> in the MutablePeerState structure, where this hashmap counts how many times each block was shared. If a block is shared with a specific peer more than twice, we could start punishing that peer.

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.

Put all Arbitrary implementations under test attribute
2 participants