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

Improve support for cycles access #3684

Merged
merged 8 commits into from
Nov 10, 2022

Conversation

zhangsoledad
Copy link
Member

@zhangsoledad zhangsoledad commented Nov 1, 2022

What problem does this PR solve?

Improve ckb's support for recording and accessing cycles as described in 3613, while implementing the fee_rate estimate short-term scheme.

What is changed and how it works?

  • For cycles and tx_size, the appending of records is done in a compatible way, so that when a new version of the node is run, new data is written, cycles and tx_size are appended, and old data is read in a compatible way. RPC returns data that is not recorded as null.
  • get_block/get_block_by_number adds a parameter with_cycles to choose whether to return cycles or not, mainly because some scenarios, such as indexer, do not need cycles and are performance sensitive, so an additional parameter is added to avoid the extra overhead needed to return cycles.

Check List

Tests

  • Unit test
  • Integration test

Release note

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

@zhangsoledad zhangsoledad changed the title Improved support for cycles access Improve support for cycles access Nov 1, 2022
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/record-cycles branch 2 times, most recently from 4c7e146 to 384f5d6 Compare November 2, 2022 02:11
@zhangsoledad zhangsoledad marked this pull request as ready for review November 2, 2022 07:24
@zhangsoledad zhangsoledad requested a review from a team as a code owner November 2, 2022 07:24
@zhangsoledad zhangsoledad requested review from quake and removed request for a team November 2, 2022 07:24
@zhangsoledad
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 10, 2022

@bors bors bot merged commit 89c5a3f into nervosnetwork:develop Nov 10, 2022
@zhangsoledad zhangsoledad deleted the zhangsoledad/record-cycles branch November 11, 2022 01:56
bors bot added a commit that referenced this pull request Nov 12, 2022
3699: fix: resolve a long reorg error r=zhangsoledad a=quake

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

resolve nervosnetwork/ckb-indexer#60

### What is changed and how it works?

remove all long fork checking code and resolve the panic error since built-in indexer always connect to the ckb node in the same chain

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test
- Integration test
- Manual test (ref: nervosnetwork/ckb-indexer#60)

### Release note <!-- Choose from None, Title Only and Note. Bugfixes orew features need a release note. -->

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



3700: fix: get block rpc compatibility r=zhangsoledad a=zhangsoledad

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

Fix #3684 introduced incompatibility issues

Because serde does not support flatten_if(serde-rs/serde#1614), and for compatibility with verbosity mode, when with_cycles specified,  the hierarchical of response object will be different.



### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test
- Integration test


### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->

```release-note
None: Exclude this PR from the release note.
```



Co-authored-by: quake <quake.wang@gmail.com>
Co-authored-by: zhangsoledad <787953403@qq.com>
bors bot added a commit that referenced this pull request Nov 12, 2022
3700: fix: get block rpc compatibility r=zhangsoledad a=zhangsoledad

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

Fix #3684 introduced incompatibility issues

Because serde does not support flatten_if(serde-rs/serde#1614), and for compatibility with verbosity mode, when with_cycles specified,  the hierarchical of response object will be different.



### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test
- Integration test


### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->

```release-note
None: Exclude this PR from the release note.
```



Co-authored-by: zhangsoledad <787953403@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants