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

fix: the bug of blobsidecars and downloader with multi-database #2564

Merged
merged 18 commits into from
Jul 16, 2024

Conversation

jingjunLi
Copy link
Contributor

@jingjunLi jingjunLi commented Jul 4, 2024

fix: some bugs for multi-database

Description

Rationale

Issue:

  1. Fixed the issue with force kill when using blob sidecars.
  2. Fixed the issue with "Failed to decode block body."
  3. Fixed the issue where some Ancient-related interface calls did not specify BlockStore() under multi-database.
  4. Fixed the issue where state-related data was not being written to stateStore in snap sync mode.

Improvement:

  1. The multidatabase flag is now only used by the init command.
  2. Refactored block meta-related interfaces, adding the MultiDatabaseReader interface. This allows reading meta interfaces without specifying blockStore, while Write/Delete related interfaces still require it.

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@jingjunLi jingjunLi changed the title fix: fix api for ReadBlobSidecarsRLP fix: some bugs for multi database Jul 5, 2024
flywukong
flywukong previously approved these changes Jul 8, 2024
Copy link
Contributor

@flywukong flywukong left a comment

Choose a reason for hiding this comment

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

LGTM

buddh0
buddh0 previously approved these changes Jul 10, 2024
@jingjunLi jingjunLi force-pushed the fix-multidatabase-sidebars branch 2 times, most recently from 62ee8af to 0aafc91 Compare July 11, 2024 09:35
RenRick
RenRick previously approved these changes Jul 16, 2024
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -34,6 +34,15 @@ import (
"golang.org/x/exp/slices"
)

// In BEP-365: Support Multi-Database Based on Data Pattern, the Chaindata will be divided into three stores: BlockStore, TrieStore, and Others,
Copy link

Choose a reason for hiding this comment

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

Multi-Database is not a BEP, pls delete "In BEP-365:"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@jingjunLi jingjunLi changed the title fix: some bugs for multi database fix: the bug of blobsidecars and downloader with multi-database Jul 16, 2024
RenRick
RenRick previously approved these changes Jul 16, 2024
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -34,6 +34,15 @@ import (
"golang.org/x/exp/slices"
)

// Support Multi-Database Based on Data Pattern, the Chaindata will be divided into three stores: BlockStore, TrieStore, and Others,
Copy link
Collaborator

Choose a reason for hiding this comment

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

TrieStore -> StateStore?
Others -> ChainStore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed comment

node/node.go Show resolved Hide resolved
zzzckck
zzzckck previously approved these changes Jul 16, 2024
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

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

LGTM

@zzzckck zzzckck merged commit 87e622e into bnb-chain:develop Jul 16, 2024
7 checks passed
flywukong added a commit to flywukong/bsc that referenced this pull request Jul 23, 2024
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.

5 participants