Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

feat: inscriber module #12

Merged
merged 25 commits into from
Aug 16, 2024
Merged

feat: inscriber module #12

merged 25 commits into from
Aug 16, 2024

Conversation

irnb
Copy link
Collaborator

@irnb irnb commented Aug 2, 2024

No description provided.

@irnb irnb marked this pull request as draft August 2, 2024 09:48
@irnb irnb marked this pull request as ready for review August 11, 2024 17:21
@irnb irnb requested a review from steph-rs August 11, 2024 17:21
steph-rs
steph-rs suggested changes Aug 12, 2024
View reviewed changes
Copy link
Collaborator

@steph-rs steph-rs left a comment

Choose a reason for hiding this comment

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

PR is great!
I dropped a few first-look comments

core/lib/via_btc_client/src/inscriber/fee.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/script_builder.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/types.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
steph-rs
steph-rs suggested changes Aug 14, 2024
View reviewed changes
Copy link
Collaborator

@steph-rs steph-rs left a comment

Choose a reason for hiding this comment

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

Looks good to me!
Let's change a few things with iteration, get rid of the cloning where it's possible, and a few more. Great job!

core/lib/via_btc_client/src/inscriber/mod.rs Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/fee.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Show resolved Hide resolved
core/lib/via_btc_client/src/inscriber/mod.rs Outdated Show resolved Hide resolved
mpavlovic-txfusion
mpavlovic-txfusion reviewed Aug 15, 2024
View reviewed changes
Copy link
Collaborator

@mpavlovic-txfusion mpavlovic-txfusion left a comment

Choose a reason for hiding this comment

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

I suggested a few code improvements.

@irnb irnb requested a review from steph-rs August 16, 2024 09:42
@irnb irnb changed the title inscriber module feat: inscriber module Aug 16, 2024
steph-rs
steph-rs approved these changes Aug 16, 2024
View reviewed changes
Copy link
Collaborator

@steph-rs steph-rs left a comment

Choose a reason for hiding this comment

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

LGTM

@steph-rs steph-rs merged commit 39cef62 into vianetwork:feat/bitcoin-integration Aug 16, 2024
2 of 3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mpavlovic-txfusion mpavlovic-txfusion mpavlovic-txfusion left review comments

@steph-rs steph-rs steph-rs approved these changes

Assignees
No one assigned
Labels
external-contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@irnb @steph-rs @mpavlovic-txfusion