-
Notifications
You must be signed in to change notification settings - Fork 108
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: special handle Bitcoin Testnet gas price estimator #2452
Conversation
WalkthroughThe recent changes introduced new functionality to handle Bitcoin gas price estimation for different network types, specifically Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2452 +/- ##
===========================================
- Coverage 69.05% 68.94% -0.12%
===========================================
Files 308 308
Lines 19192 19223 +31
===========================================
Hits 13253 13253
- Misses 5264 5295 +31
Partials 675 675
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
changelog.md (1)
92-92
: Fix typographical issue: Add hyphen in "cherry-picked".The term "cherry picked" should be hyphenated as "cherry-picked".
- * [2396](https://github.com/zeta-chain/node/issues/2386) - special handle bitcoin testnet gas price estimator + * [2396](https://github.com/zeta-chain/node/issues/2386) - special handle bitcoin testnet gas price estimator
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- changelog.md (1 hunks)
- zetaclient/chains/bitcoin/observer/observer.go (3 hunks)
- zetaclient/chains/bitcoin/rpc/rpc.go (2 hunks)
- zetaclient/chains/bitcoin/rpc/rpc_live_test.go (9 hunks)
Additional context used
Path-based instructions (3)
zetaclient/chains/bitcoin/rpc/rpc.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/rpc/rpc_live_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
zetaclient/chains/bitcoin/rpc/rpc.go
[warning] 122-125: zetaclient/chains/bitcoin/rpc/rpc.go#L122-L125
Added lines #L122 - L125 were not covered by tests
[warning] 129-130: zetaclient/chains/bitcoin/rpc/rpc.go#L129-L130
Added lines #L129 - L130 were not covered by tests
[warning] 132-134: zetaclient/chains/bitcoin/rpc/rpc.go#L132-L134
Added lines #L132 - L134 were not covered by tests
[warning] 136-138: zetaclient/chains/bitcoin/rpc/rpc.go#L136-L138
Added lines #L136 - L138 were not covered by tests
[warning] 142-144: zetaclient/chains/bitcoin/rpc/rpc.go#L142-L144
Added lines #L142 - L144 were not covered by tests
[warning] 146-147: zetaclient/chains/bitcoin/rpc/rpc.go#L146-L147
Added lines #L146 - L147 were not covered by tests
[warning] 152-153: zetaclient/chains/bitcoin/rpc/rpc.go#L152-L153
Added lines #L152 - L153 were not covered by tests
[warning] 157-157: zetaclient/chains/bitcoin/rpc/rpc.go#L157
Added line #L157 was not covered by testszetaclient/chains/bitcoin/observer/observer.go
[warning] 346-347: zetaclient/chains/bitcoin/observer/observer.go#L346-L347
Added lines #L346 - L347 were not covered by tests
[warning] 638-641: zetaclient/chains/bitcoin/observer/observer.go#L638-L641
Added lines #L638 - L641 were not covered by tests
[warning] 644-646: zetaclient/chains/bitcoin/observer/observer.go#L644-L646
Added lines #L644 - L646 were not covered by tests
[warning] 648-652: zetaclient/chains/bitcoin/observer/observer.go#L648-L652
Added lines #L648 - L652 were not covered by tests
[warning] 654-655: zetaclient/chains/bitcoin/observer/observer.go#L654-L655
Added lines #L654 - L655 were not covered by tests
[warning] 659-661: zetaclient/chains/bitcoin/observer/observer.go#L659-L661
Added lines #L659 - L661 were not covered by tests
[warning] 664-664: zetaclient/chains/bitcoin/observer/observer.go#L664
Added line #L664 was not covered by tests
LanguageTool
changelog.md
[misspelling] ~89-~89: This word is normally spelled with a hyphen.
Context: ...//pull/2327) - partially cherry picked the fix to Bitcoin outbound dust amount...(EN_COMPOUNDS_CHERRY_PICKED)
Additional comments not posted (3)
zetaclient/chains/bitcoin/rpc/rpc.go (1)
18-22
: Constants look good.The constants
feeRateCountBackBlocks
anddefaultTestnetFeeRate
are clear and appropriate for their intended use.zetaclient/chains/bitcoin/rpc/rpc_live_test.go (2)
Line range hint
94-112
:
Function changes look good.The
createRPCClient
function improvements enhance maintainability by using chain IDs from thechains
package instead of hardcoding values.
420-430
: Test function looks good.The
LiveTestGetRecentFeeRate
function is straightforward and includes necessary assertions to validate the results.
👏🏻👏🏻 |
…Price in single spot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- zetaclient/chains/bitcoin/observer/observer.go (3 hunks)
- zetaclient/chains/bitcoin/rpc/rpc.go (2 hunks)
Additional context used
Path-based instructions (2)
zetaclient/chains/bitcoin/rpc/rpc.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
zetaclient/chains/bitcoin/rpc/rpc.go
[warning] 123-126: zetaclient/chains/bitcoin/rpc/rpc.go#L123-L126
Added lines #L123 - L126 were not covered by tests
[warning] 130-131: zetaclient/chains/bitcoin/rpc/rpc.go#L130-L131
Added lines #L130 - L131 were not covered by tests
[warning] 133-135: zetaclient/chains/bitcoin/rpc/rpc.go#L133-L135
Added lines #L133 - L135 were not covered by tests
[warning] 137-139: zetaclient/chains/bitcoin/rpc/rpc.go#L137-L139
Added lines #L137 - L139 were not covered by tests
[warning] 143-145: zetaclient/chains/bitcoin/rpc/rpc.go#L143-L145
Added lines #L143 - L145 were not covered by tests
[warning] 147-148: zetaclient/chains/bitcoin/rpc/rpc.go#L147-L148
Added lines #L147 - L148 were not covered by tests
[warning] 153-154: zetaclient/chains/bitcoin/rpc/rpc.go#L153-L154
Added lines #L153 - L154 were not covered by tests
[warning] 158-158: zetaclient/chains/bitcoin/rpc/rpc.go#L158
Added line #L158 was not covered by testszetaclient/chains/bitcoin/observer/observer.go
[warning] 345-346: zetaclient/chains/bitcoin/observer/observer.go#L345-L346
Added lines #L345 - L346 were not covered by tests
[warning] 349-350: zetaclient/chains/bitcoin/observer/observer.go#L349-L350
Added lines #L349 - L350 were not covered by tests
[warning] 352-352: zetaclient/chains/bitcoin/observer/observer.go#L352
Added line #L352 was not covered by tests
[warning] 355-355: zetaclient/chains/bitcoin/observer/observer.go#L355
Added line #L355 was not covered by tests
[warning] 357-357: zetaclient/chains/bitcoin/observer/observer.go#L357
Added line #L357 was not covered by tests
[warning] 359-359: zetaclient/chains/bitcoin/observer/observer.go#L359
Added line #L359 was not covered by tests
[warning] 362-363: zetaclient/chains/bitcoin/observer/observer.go#L362-L363
Added lines #L362 - L363 were not covered by tests
[warning] 365-366: zetaclient/chains/bitcoin/observer/observer.go#L365-L366
Added lines #L365 - L366 were not covered by tests
[warning] 368-368: zetaclient/chains/bitcoin/observer/observer.go#L368
Added line #L368 was not covered by tests
[warning] 378-378: zetaclient/chains/bitcoin/observer/observer.go#L378
Added line #L378 was not covered by tests
[warning] 380-380: zetaclient/chains/bitcoin/observer/observer.go#L380
Added line #L380 was not covered by tests
[warning] 647-649: zetaclient/chains/bitcoin/observer/observer.go#L647-L649
Added lines #L647 - L649 were not covered by tests
[warning] 651-655: zetaclient/chains/bitcoin/observer/observer.go#L651-L655
Added lines #L651 - L655 were not covered by tests
[warning] 657-659: zetaclient/chains/bitcoin/observer/observer.go#L657-L659
Added lines #L657 - L659 were not covered by tests
Additional comments not posted (3)
zetaclient/chains/bitcoin/rpc/rpc.go (1)
123-159
: Function implementation looks good.The
GetRecentFeeRate
function is well-structured and includes necessary error handling.Tools
GitHub Check: codecov/patch
[warning] 123-126: zetaclient/chains/bitcoin/rpc/rpc.go#L123-L126
Added lines #L123 - L126 were not covered by tests
[warning] 130-131: zetaclient/chains/bitcoin/rpc/rpc.go#L130-L131
Added lines #L130 - L131 were not covered by tests
[warning] 133-135: zetaclient/chains/bitcoin/rpc/rpc.go#L133-L135
Added lines #L133 - L135 were not covered by tests
[warning] 137-139: zetaclient/chains/bitcoin/rpc/rpc.go#L137-L139
Added lines #L137 - L139 were not covered by tests
[warning] 143-145: zetaclient/chains/bitcoin/rpc/rpc.go#L143-L145
Added lines #L143 - L145 were not covered by tests
[warning] 147-148: zetaclient/chains/bitcoin/rpc/rpc.go#L147-L148
Added lines #L147 - L148 were not covered by tests
[warning] 153-154: zetaclient/chains/bitcoin/rpc/rpc.go#L153-L154
Added lines #L153 - L154 were not covered by tests
[warning] 158-158: zetaclient/chains/bitcoin/rpc/rpc.go#L158
Added line #L158 was not covered by testszetaclient/chains/bitcoin/observer/observer.go (2)
345-380
: Function changes look good.The
PostGasPrice
function correctly delegates special handling to the newspecialHandleFeeRate
function for non-mainnet network types.Tools
GitHub Check: codecov/patch
[warning] 345-346: zetaclient/chains/bitcoin/observer/observer.go#L345-L346
Added lines #L345 - L346 were not covered by tests
[warning] 349-350: zetaclient/chains/bitcoin/observer/observer.go#L349-L350
Added lines #L349 - L350 were not covered by tests
[warning] 352-352: zetaclient/chains/bitcoin/observer/observer.go#L352
Added line #L352 was not covered by tests
[warning] 355-355: zetaclient/chains/bitcoin/observer/observer.go#L355
Added line #L355 was not covered by tests
[warning] 357-357: zetaclient/chains/bitcoin/observer/observer.go#L357
Added line #L357 was not covered by tests
[warning] 359-359: zetaclient/chains/bitcoin/observer/observer.go#L359
Added line #L359 was not covered by tests
[warning] 362-363: zetaclient/chains/bitcoin/observer/observer.go#L362-L363
Added lines #L362 - L363 were not covered by tests
[warning] 365-366: zetaclient/chains/bitcoin/observer/observer.go#L365-L366
Added lines #L365 - L366 were not covered by tests
[warning] 368-368: zetaclient/chains/bitcoin/observer/observer.go#L368
Added line #L368 was not covered by tests
[warning] 378-378: zetaclient/chains/bitcoin/observer/observer.go#L378
Added line #L378 was not covered by tests
[warning] 380-380: zetaclient/chains/bitcoin/observer/observer.go#L380
Added line #L380 was not covered by tests
647-661
: Function implementation looks good.The
specialHandleFeeRate
function is well-structured and includes necessary error handling.Tools
GitHub Check: codecov/patch
[warning] 647-649: zetaclient/chains/bitcoin/observer/observer.go#L647-L649
Added lines #L647 - L649 were not covered by tests
[warning] 651-655: zetaclient/chains/bitcoin/observer/observer.go#L651-L655
Added lines #L651 - L655 were not covered by tests
[warning] 657-659: zetaclient/chains/bitcoin/observer/observer.go#L657-L659
Added lines #L657 - L659 were not covered by tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- zetaclient/chains/bitcoin/observer/observer.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- zetaclient/chains/bitcoin/observer/observer.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Description
Closes: 2386
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation