-
Notifications
You must be signed in to change notification settings - Fork 48
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
Associate block with metadata and fix transaction encoding #2045
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some quick comments. Perhaps I'll post more later when I have time. We'll definitely need feedback from @jbearer here.
…fic types, update return types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the overall shape of things is right now. All of my comments are just details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks good, except see this thread about possibly removing the commit()
method of BlockPayload
and having HotShot keep track of the commit on its own. WDYT about that @shenkeyao @ggutoski
@jbearer As mentioned in that thread, I think it makes sense to remove the |
BlockPayload
trait.Metadata
type and a constructor to build the payload and metadata.BlockHeader
constructor to takemetadata
as an input.BlockPayload
functions for theVIDBlockPayload
struct.Closes #2024. Closes #2025.