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

require SerializedProgram.from_bytes() is passed a valid clvm structure #16674

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Oct 23, 2023

Purpose:

The Streamable protocol (normally) requires input passed to parse() and from_bytes() be equally valid input. The distinction is that parse() consumes input from a stream, whereas from_bytes() expects the complete byte buffer to represent the complete value to be consumed for the streamable type.

SerializedProgram is currently an exception to this, where there is no validation at all of the input passed to from_bytes(), whereas parse() does validate the input. In most normal production use, parse() is how SerializedPrograms are initialized (such as parsing a FullBlock from the network or database).

There are however a few tests where we initialize SerializedProgram with invalid input. This is a small tweak to correct this quirk, and fix the tests.

Current Behavior:

SerializedProgram.from_bytes(b"aaa") is valid.

New Behavior:

SerializedProgram.from_bytes(b"aaa") is not valid.

@arvidn arvidn requested a review from a team as a code owner October 23, 2023 23:36
@arvidn arvidn added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Cleanup Code cleanup labels Oct 23, 2023
@arvidn arvidn force-pushed the serialized-program-from-bytes branch from 04cb052 to e3fd2b9 Compare October 23, 2023 23:56
@arvidn arvidn requested a review from altendky October 24, 2023 00:17
@coveralls-official
Copy link

Pull Request Test Coverage Report for Build 6620383299

  • 9 of 9 (100.0%) changed or added relevant lines in 4 files are covered.
  • 104 unchanged lines in 12 files lost coverage.
  • Overall coverage decreased (-0.09%) to 90.083%

Files with Coverage Reduction New Missed Lines %
chia/data_layer/dl_wallet_store.py 1 95.74%
chia/rpc/wallet_rpc_api.py 1 88.06%
chia/server/node_discovery.py 1 78.01%
chia/wallet/wallet_node.py 1 87.8%
chia/timelord/timelord_api.py 2 87.85%
chia/farmer/farmer_api.py 3 86.35%
chia/rpc/rpc_server.py 3 87.33%
chia/server/server.py 6 81.07%
chia/full_node/full_node.py 8 86.3%
chia/full_node/full_node_api.py 9 77.33%
Totals Coverage Status
Change from base Build 6616417205: -0.09%
Covered Lines: 93077
Relevant Lines: 103270

💛 - Coveralls

@cmmarslender cmmarslender merged commit 493d36b into main Oct 24, 2023
250 checks passed
@cmmarslender cmmarslender deleted the serialized-program-from-bytes branch October 24, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Cleanup Code cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants