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

Remove the coin amount from state layer solution #18500

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

Rigidity
Copy link
Contributor

@Rigidity Rigidity commented Aug 20, 2024

Purpose:

Removes the coin amount (typically 1) from the end of the NFT state layer (referred to here as "metadata" layer) solution. The actual puzzle's solution does not contain more than one argument, the inner solution.

The way it is now increases the cost slightly, and breaks code which expects an exact format for the solution. That said, code which does can likely be relaxed a bit to adapt to this.

@Rigidity Rigidity requested a review from a team as a code owner August 20, 2024 05:22
@Rigidity Rigidity added Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog NFT Wallet labels Aug 20, 2024
@Yakuhito
Copy link
Contributor

LGTM

Copy link

Pull Request Test Coverage Report for Build 10465812450

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.04%) to 90.872%

Files with Coverage Reduction New Missed Lines %
chia/wallet/wallet_node.py 1 89.01%
chia/daemon/client.py 1 73.94%
chia/server/server.py 2 82.21%
chia/server/ws_connection.py 2 88.69%
Totals Coverage Status
Change from base Build 10461877247: 0.04%
Covered Lines: 101367
Relevant Lines: 111524

💛 - Coveralls

Copy link
Contributor

@Quexington Quexington left a comment

Choose a reason for hiding this comment

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

This seems fine, I would imagine the amount was necessary in a previous iteration and we simply forgot to update the drivers when we iterated.

@Rigidity Rigidity added the ready_to_merge Submitter and reviewers think this is ready label Aug 20, 2024
@pmaslana pmaslana merged commit 2fd02d5 into main Sep 4, 2024
375 checks passed
@pmaslana pmaslana deleted the remove-state-layer-amount branch September 4, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog NFT ready_to_merge Submitter and reviewers think this is ready Wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants