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

feat(): add NewPeek / RequestBlock / RespondBlock #118

Merged
merged 8 commits into from
Mar 22, 2024

Conversation

n33pm
Copy link
Contributor

@n33pm n33pm commented Mar 20, 2024

Add FullNode Message Types:

  • NewPeak
  • RequestBlock
  • RespondBlock

Types:

  • add streamable

Streamable:

  • unmarshal / marshal Array
  • unmarshal / marshal Struct
  • unmarshal / marshal Bool

@n33pm n33pm force-pushed the main branch 2 times, most recently from 4268cbc to 6ef38c4 Compare March 20, 2024 14:12
@n33pm n33pm marked this pull request as ready for review March 20, 2024 14:12
@cmmarslender
Copy link
Contributor

cmmarslender commented Mar 20, 2024

Thanks for the PR! I'll take a more in depth look soon. I know tests are lacking across a lot of the project, but if you are able, it would be great to have tests for more of the Streamable implementations in here, to make sure someone reordering structs, etc in the future doesn't break the serialized format (since the order is critical when dealing with streamable types). Seems like there are tests and I missed at first, doing the review now

Copy link
Contributor

@cmmarslender cmmarslender left a comment

Choose a reason for hiding this comment

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

Thanks for putting all the effort here in getting this added. This was probably one of the bigger initial undertakings for better streamable support.

I have a bunch of requested changes - A majority of them are just suggestions that should apply easily in GitHub to update doc references, since most of these originates in chia-rs these days.

Aside from those, just a few other minor changes requested

pkg/peerprotocol/fullnode.go Outdated Show resolved Hide resolved
pkg/types/block.go Outdated Show resolved Hide resolved
pkg/types/block.go Outdated Show resolved Hide resolved
pkg/types/classgroup.go Outdated Show resolved Hide resolved
pkg/types/endofslotbundle.go Outdated Show resolved Hide resolved
pkg/types/vdf.go Outdated Show resolved Hide resolved
pkg/types/vdf.go Outdated Show resolved Hide resolved
pkg/streamable/streamable.go Show resolved Hide resolved
pkg/streamable/streamable.go Show resolved Hide resolved
pkg/protocols/fullnode.go Outdated Show resolved Hide resolved
@cmmarslender cmmarslender merged commit 993395c into Chia-Network:main Mar 22, 2024
4 checks passed
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.

2 participants