Add a less strict solution representation #674
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale
When parsing CLVM solutions currently, the nil terminator is strict since they are represented as a normal list. This is fine if the solution is structured correctly, but as seen in Chia-Network/chia-blockchain#18500, this is not always the case. I had an NFT which failed to parse due to this mistake of including the coin amount in the NFT state layer solution.
How does this fix the issue
Instead of requiring the solution's terminator (which here refers to the last CLVM pair's "rest" value after extracting all of the required values in the list) to be nil, it can be any value and is not checked. This is achieved by adding a new representation
solution
which is identical tolist
except with this extra bit of flexibility. When serialized, it still includes the nil terminator as it should.Side effects
The
SingletonSolution
type is used in the fast-forward feature, so it's a consideration whether allowing this flexibility makes sense for that mempool logic. If not, a separateStrictSingletonSolution
could be created with the representationlist
, for use cases like that. It's not clear to me whether this is important though.