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

[Bug] Types Confusion in Source and Docs #738

Open
buggybuck opened this issue Dec 29, 2024 · 6 comments
Open

[Bug] Types Confusion in Source and Docs #738

buggybuck opened this issue Dec 29, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@buggybuck
Copy link

What happened?

Hey there!

I'm a little confused by how some of the types have different names in the source code [1] and in the documentation [2].

You find 'register_interest_in_puzzle_hash' (enum: 70) in the source but not in the docs. Instead you find 'register_for_ph_updates' in the docs which in turn has no enum in the source.

'register_interest_in_coin' (enum: 72) <-> 'register_for_coin_updates'. Same situation.

'respond_to_ph_update' (enum: 71) <-> 'respond_to_ph_updates'. It's plural in the docs.

'respond_to_coin_update' (enum: 73) <-> 'respond_to_coin_updates'.

Is there a system to the madness? :D

[1] chia-blockchain/chia/protocols/protocol_message_types.py
[2] docs.chia.net

Version

main

What platform are you using?

Linux

What ui mode are you using?

GUI

Relevant log output

No response

@buggybuck buggybuck added the bug Something isn't working label Dec 29, 2024
@wjblanke
Copy link

wjblanke commented Jan 8, 2025

Also request_block_header(s) Brandt see JIRA

@BrandtH22
Copy link
Collaborator

The wallet protocol documentation (here: https://docs.chia.net/wallet-protocol/) documents (or, at least, mentions) the message request_block_header and respond_block_header. Those messages are deprecated and replaced by request_block_headers (plural) and respond_block_headers.

The new messages should be documented

It should be mentioned that the old messages are deprecated

It should be documented whether orphan blocks are expected to be returned (I believe that’s not the intention, and fixing this in the node)

@BrandtH22
Copy link
Collaborator

Moving this to the Docs repo for better tracking

@BrandtH22 BrandtH22 transferred this issue from Chia-Network/chia-blockchain Jan 8, 2025
@BrandtH22
Copy link
Collaborator

BrandtH22 commented Jan 13, 2025

Hey @buggybuck , we are working on cleaning up the wallet protocol docs in this pr:
https://github.com/Chia-Network/chia-docs/pull/740/files#diff-fbcad952ed2e42f9e6c0b2e65fa3b9afcc25e277765a461c33d92d23b4bf149a

Do take note though that the docs site is accurate regarding the messages you have listed in this ticket. The confusion arises from the part of the chia codebase that is being reviewed. The file protocol_message_types is using outdated messages and wrapping them to integrate the new messages.

The source code that contains the most up to date information on wallet_protocol messages is the wallet_protocol file (https://github.com/Chia-Network/chia-blockchain/blob/main/chia/protocols/wallet_protocol.py) also linked at the top of the docs page for the wallet protocol.

I have reached out to the dev team to see if we can update the protocol_message_types file to align with the currently used messages but it might be some time to clear out the tech debt leading to that difference (this is the pr for that change: Chia-Network/chia-blockchain#19132).

As a side note, some of the wallet protocol messages are no longer using the python codebase (ex. respond_to_ph_updates) and are instead using the rust codebase here (https://github.com/Chia-Network/chia_rs/blob/f57de20c896a5297c51ad36a057694b2ec94a087/crates/chia-protocol/src/wallet_protocol.rs#L159).

As we migrate everything from python to rust you will see more messages make that switch and we will make sure to add that rust repo to the docs site for reference.

This was a lot of info, do you have any followup questions or concerns? (note I will update here once the pr is in place and the docs site is updated)

@buggybuck
Copy link
Author

Hi BrandtH22! Thank you for your explanation and the pointers! Much appreciated!

@buggybuck
Copy link
Author

Hi there!

Here is another mismatching pair: request_ses_hashes (76) / respond_ses_hashes (77)
In the docs we find references to: request_ses_info / respond_ses_info

Kind regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants