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

feat(rpc): Introduce the new method generate_epochs in the IntegrationTest module #4128

Merged
merged 9 commits into from
Sep 9, 2023

Conversation

EthanYuan
Copy link
Collaborator

@EthanYuan EthanYuan commented Aug 31, 2023

What problem does this PR solve?

Problem Summary:

In the context of CKB applications, many contracts are closely tied to epoch processes, including "DAO" script, "cheque" script, and others. During the development process, facilitating the acceleration of epochs becomes essential.

Before this PR, there were two methods:

  1. relied on miners rapidly producing blocks
  2. required multiple calls to the CKB RPC generate_block

Clearly, the latter is more suitable for integration testing (without requiring a miner). However, using generate_block poses two issues. Firstly, it requires the calling side to calculate and determine the number of times generate_block should be called based on the desired epochs to fast-forward. Secondly, if called too quickly, generate_block may produce duplicate block hashes (indicating that no new blocks were generated), necessitating deduplication on the calling side.

The newly designed RPC generate_epochs introduces a parameter called num_epochs. Upon successful execution, it will return the current epoch target (in the form of EpochNumberWithFraction), significantly simplifying integration testing for applications like "DAO".

What is changed and how it works?

What's Changed:

add rpc:

#[rpc(name = "generate_epochs")]
fn generate_epochs(&self, num_epochs: EpochNumberWithFraction) -> Result<EpochNumberWithFraction>;

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
  • No code ci-runs-only: [ quick_checks,linters ]

Release note

Title Only: Include only the PR title in the release note.

@EthanYuan EthanYuan requested a review from a team as a code owner August 31, 2023 07:32
@EthanYuan EthanYuan requested review from doitian and quake and removed request for a team August 31, 2023 07:32
@EthanYuan EthanYuan changed the title feat(rpc): Introduce the new method fast_forward_epochs in the IntegrationTest module feat(rpc): Introduce the new method generate_epochs in the IntegrationTest module Sep 6, 2023
@eval-exec eval-exec added the t:enhancement Type: Feature, refactoring. label Sep 7, 2023
rpc/src/module/test.rs Outdated Show resolved Hide resolved
@doitian doitian added this pull request to the merge queue Sep 9, 2023
Merged via the queue into nervosnetwork:develop with commit 2c7fce6 Sep 9, 2023
33 checks passed
@EthanYuan EthanYuan deleted the fast-forward-epochs branch September 11, 2023 01:51
@doitian doitian mentioned this pull request Oct 9, 2023
@homura homura mentioned this pull request Oct 13, 2023
4 tasks
"id": 42,
"jsonrpc": "2.0",
"result": "0xa0001000003",
"error": null
Copy link
Contributor

Choose a reason for hiding this comment

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

The actual request response does not have "error": null

ckb@test-02:~/xueyl/ckb/ckb/generate_epochs/specs$ curl --location 'http://127.0.0.1:8202' \
> --header 'Content-Type: application/json' \
> --data '{
> "id": 42,
> "jsonrpc": "2.0",
> "method": "generate_epochs",
> "params": ["0x7080000000002"]
> }'
{"jsonrpc":"2.0","result":"0xa0000000004","id":42}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems that similar issues exist in other RPC method examples, and we can consider removing them together.

fn generate_epochs(
&self,
num_epochs: EpochNumberWithFraction,
) -> Result<EpochNumberWithFraction> {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the request growth epoch is too large, the waiting time will be too long. The more logs generated, the higher the CPU load. Let’s see if we can optimize the CPU usage through asynchronous means.

ckb@test-02:~/xueyl/ckb/ckb/generate_epochs/specs$ curl --location 'http://127.0.0.1:8202/' --header 'Content-Type: application/json' --data '{
  "id": 42,
  "jsonrpc": "2.0",
  "method": "generate_epochs",
  "params": ["0xFFFFFFFFFFFFFFFF"]
}'

PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
2624591 ckb 20 0 1811504 782352 60596 S 360.8 2.4 47:56.19 ckb

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feature is only enabled in the development chain, and it doesn't make any sense for developers to set too large a value. It is a low priority to optimize for such usage scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:enhancement Type: Feature, refactoring.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants