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

Type discrepancy in DeploymentInfo between codebase and rpc/README.md documentation #3951

Closed
eval-exec opened this issue Apr 23, 2023 · 7 comments · Fixed by #3954
Closed
Labels
t:bug Type: This doesn't seem right.

Comments

@eval-exec
Copy link
Collaborator

eval-exec commented Apr 23, 2023

The type of threshold in DeploymentInfo is defined as Ratio in the codebase

pub struct DeploymentInfo {
/// determines which bit in the `version` field of the block is to be used to signal the softfork lock-in and activation.
/// It is chosen from the set {0,1,2,...,28}.
pub bit: u8,
/// specifies the first epoch in which the bit gains meaning.
pub start: EpochNumber,
/// specifies an epoch at which the miner signaling ends.
/// Once this epoch has been reached,
/// if the softfork has not yet locked_in (excluding this epoch block's bit state),
/// the deployment is considered failed on all descendants of the block.
pub timeout: EpochNumber,
/// specifies the epoch at which the softfork is allowed to become active.
pub min_activation_epoch: EpochNumber,
/// the length in epochs of the signalling period
pub period: EpochNumber,
/// the ratio of blocks with the version bit set required to activate the feature
pub threshold: Ratio,
/// The first epoch which the current state applies
pub since: EpochNumber,
/// With each epoch and softfork, we associate a deployment state. The possible states are:
/// * `DEFINED` is the first state that each softfork starts. The blocks of 0 epoch is by definition in this state for each deployment.
/// * `STARTED` for all blocks reach or past the start_epoch.
/// * `LOCKED_IN` for one period after the first period with STARTED blocks of which at least threshold has the associated bit set in version.
/// * `ACTIVE` for all blocks after the LOCKED_IN period.
/// * `FAILED` for all blocks after the timeout_epoch, if LOCKED_IN was not reached.
pub state: DeploymentState,
}
.

However, in the rpc/README.md DeploymentInfo file, it is documented as being of type EpochNumber, and the since field is not yet documented for DeploymentInfo.

It appears that there may be an issue with the ./devtools/doc/rpc.py file.

@eval-exec eval-exec added the t:bug Type: This doesn't seem right. label Apr 23, 2023
@eval-exec
Copy link
Collaborator Author

In this PR , I generate right rpc doc, but it didn't pass the CI job. Also, there appears to be an issue with the CI action.

@chenyukang
Copy link
Collaborator

The line of 110 could be removed?

https://github.com/nervosnetwork/ckb/blob/develop/Makefile#L110-L111

@chenyukang
Copy link
Collaborator

Seems there is an issue in cargo doc, note the ratio part:

image

Use cargo +nightly doc to generate the doc html is OK.

@eval-exec
Copy link
Collaborator Author

eval-exec commented Apr 23, 2023

I think we can use cargo doc --workspace to generate all workspace members, this will generate right cargo doc.

@chenyukang
Copy link
Collaborator

chenyukang commented Apr 23, 2023

cargo +nightly will change the format of HTML documents, which introduces a lot of differences rpc/README.md, there is some existing code to handle this kind of issue:

# after 1.56 rustdoc change relative link

HTML is prone to change, we'd better use another solution to generate rpc/README.md in the future.

A possible solution is to use rustdoc to generate json format, such as:

cargo +nightly rustdoc  -p ckb-fixed-hash  -- -Z unstable-options --output-format json

Out of curiosity, why do we generate rpc/README.md?

@eval-exec
Copy link
Collaborator Author

eval-exec commented Apr 23, 2023

why do we generate rpc/README.md?

Cc. @zhangsoledad

I think we can view RPC modules to learn what APIs we have.

@doitian
Copy link
Member

doitian commented Apr 24, 2023

Out of curiosity, why do we generate rpc/README.md?

Make document close to the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:bug Type: This doesn't seem right.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants