Skip to content

Add fiber-types crate#1152

Merged
quake merged 23 commits intonervosnetwork:developfrom
chenyukang:yukang-add-fiber-types
Mar 4, 2026
Merged

Add fiber-types crate#1152
quake merged 23 commits intonervosnetwork:developfrom
chenyukang:yukang-add-fiber-types

Conversation

@chenyukang
Copy link
Collaborator

Separate types from fiber-lib.

ref to: fiber-types Crate Design

@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 377311f to 130441e Compare February 28, 2026 04:31
@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 130441e to 030d400 Compare February 28, 2026 04:39
@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 1b78f04 to c47df1e Compare March 3, 2026 12:38
@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from c47df1e to 4e69aeb Compare March 3, 2026 13:18
@quake quake requested a review from Copilot March 4, 2026 00:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.


impl InMemorySigner {
pub fn generate_from_seed(params: &[u8]) -> Self {
impl InMemorySignerExt for InMemorySigner {
Copy link
Member

Choose a reason for hiding this comment

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

can we keep InMemorySigner in fiber-lib crate?

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's stored in db by a type chain:

Type `InMemorySigner` is STORE-RELATED (reachable from KeyValue).

Defined in: /Users/yukang/code/fiber/crates/fiber-types/src/channel.rs
Dependency chain(s) from KeyValue:
  KeyValue::ChannelActorState -> ChannelActorState -> ChannelActorData -> InMemorySigner

molecule = { version = "0.8.0", default-features = false }

ckb-hash = "1"
bitcoin = { version = "0.32", features = ["serde"] }
Copy link
Member

Choose a reason for hiding this comment

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

we only use sha256 related fn with this crate , a minimal sha256 crate may be better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we only use bitcoin::hashes::sha256::Hash for type conversion (in primitives.rs and invoice.rs). This conversion is required for interoperability with lightning-invoice, which itself depends on Bitcoin. So even if we remove the direct dependency, it will still be brought in transitively via lightning-invoice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, we can use minimal bitcoin::hashes::sha256.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

whether we should add a feature gate such as cch .

@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 446fe66 to 5729184 Compare March 4, 2026 01:11
@quake quake requested a review from Copilot March 4, 2026 01:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from f2e0cff to 1a6ef43 Compare March 4, 2026 01:36
@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 8a7f645 to 2cad1f3 Compare March 4, 2026 03:49
@quake quake requested a review from Copilot March 4, 2026 04:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@chenyukang chenyukang force-pushed the yukang-add-fiber-types branch from 98bab7e to 92fbd54 Compare March 4, 2026 06:57
@quake quake merged commit ab12c41 into nervosnetwork:develop Mar 4, 2026
35 checks passed
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.

4 participants