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

Binary Serialization Standard #1478

Conversation

ACStone-MTS
Copy link
Contributor

What does this PR fix/introduce?

This PR turns the "Serialization Standard" into the "Binary Serialization Standard" and splits it into multiple pages for ease of reading.

The document itself now includes all updated Condor types outside of the chainspec, which will be a separate PR.

Closes #1471 #1378

Additional context

// Add further detail on the current situation, or what the PR seeks to address.
// Include loom, screenshots, or gifs(record, compress) if helpful.

Checklist

(Delete any that aren't relevant)

  • Docs are successfully building - yarn install && yarn run build.
  • For new internal links I used relative file paths (with .md extension) - e.g. ../../faq/faq-general.md - instead of introducing absolute file path, or relative/absolute URL.
  • All external links have been verified with yarn run check:externals.
  • My changes follow the Casper docs style guidelines.
  • All technical procedures have been tested (if you want help with this, mention it in Reviewers).
  • If structural changes are introduced (not just content changes), cross-broswer testing has been completed.

Reviewers

TBD

@ACStone-MTS ACStone-MTS added the v2.0 Merge to feat-2.0_docs label Jul 5, 2024
@ACStone-MTS ACStone-MTS self-assigned this Jul 5, 2024
@ACStone-MTS
Copy link
Contributor Author

@bradjohnl I'm getting a strange failure on the CICD for this one - could you take a look when you get a chance?

@bradjohnl
Copy link
Contributor

bradjohnl commented Jul 8, 2024

@bradjohnl I'm getting a strange failure on the CICD for this one - could you take a look when you get a chance?

Hey @ACStoneCL . It's fixed. I've removed some hardcoded, and apparently redundant tests that were targeting pages that this PR removed / changed.

I've created an internal issue in our SRE board to check the testing strategy and make sure we are already testing each internal and external link with other processes (namely the: node check-external-urls.js test script).

Copy link
Collaborator

@ipopescu ipopescu left a comment

Choose a reason for hiding this comment

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

I left a few minor nits and a few questions about some methods I don't see anymore. It would be good to address these before approving.

While reviewing this file, a few other questions came up:

  • There's a bit of duplication with the json-rpc/types_chain.md file, although the serialization section is much more detailed. Perhaps we can auto-generate the json-rpc docs in the future to avoid double maintenance.
  • I see that some types are missing from what we expose in the JSON RPC; perhaps this is because they are not exposed in the RPC methods. Do you know why this the case?
    • Some of the fields I noted were: Message, MessageAddr, MessageLimits, MessagePayload, MessageTopicOperation, DeployConfig, TransactionConfig, TransactionHeader, TransactionId, TransactionWithExecutionInfo, BalanceHoldAddr, BidAddr, BlockGlobalAddr, BlockSignatures.

source/docs/casper/concepts/serialization/index.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/primitives.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/primitives.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/primitives.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/primitives.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/types.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/types.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/types.md Outdated Show resolved Hide resolved
source/docs/casper/concepts/serialization/types.md Outdated Show resolved Hide resolved
@ipopescu
Copy link
Collaborator

I tried to help resolve a conflict, and then I had to fix a link for the build to pass.

Copy link
Contributor

@darthsiroftardis darthsiroftardis left a comment

Choose a reason for hiding this comment

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

Approval pending comments from @ipopescu

@ACStone-MTS ACStone-MTS merged commit a55d8b7 into casper-network:feat-2.0_docs Aug 15, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.0 Merge to feat-2.0_docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants