ci: seperate linting into its own action#409
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a new GitHub Actions workflow dedicated to linting a Go project. The The codebase includes multiple refactorings where conditional Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Runner as Runner Environment
participant Checkout as Checkout Action
participant SetupGo as Setup Go Action
participant GolangciLint as GolangCI Lint Action
GitHub ->> Runner: Trigger on push/PR to master
Runner ->> Checkout: Checkout code
Checkout -->> Runner: Code ready
Runner ->> SetupGo: Install Go 1.23.x
SetupGo -->> Runner: Go environment ready
Runner ->> GolangciLint: Run lint checks
GolangciLint -->> Runner: Lint results
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (37)
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/workflows/lint.yml (5)
3-8: Trigger Configuration Validation
The workflow is triggered on pushes to themasterbranch as well as on pull requests. Verify that pull request events do not require additional branch filters or conditions based on your project requirements.
16-16: Action Version Consideration for Checkout
The workflow usesactions/checkout@v3, but static analysis indicates that this version might be outdated. Consider updating to a newer version (if available, e.g.,actions/checkout@v4) to ensure improved performance and compatibility.🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
19-19: Update Setup-Go Action Version
The configuration employsactions/setup-go@v4, yet the static analysis tool flags this version as potentially outdated. Please verify if a newer version (for example,actions/setup-go@v5) is available and update the workflow accordingly.🧰 Tools
🪛 actionlint (1.7.4)
19-19: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: Upgrade golangci-lint-action Version
Static analysis indicates thatgolangci/golangci-lint-action@v3may be outdated. Consider upgrading to the latest version of this action for optimal linting performance and to benefit from recent fixes and features.🧰 Tools
🪛 actionlint (1.7.4)
24-24: the runner of "golangci/golangci-lint-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
26-26: Fix Formatting Issues
There are trailing spaces and a missing newline at the end of the file. Please remove the trailing spaces and ensure the file ends with a newline to comply with YAML formatting best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
[error] 26-26: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/lint.yml(1 hunks).github/workflows/test.yml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/lint.yml
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
19-19: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: the runner of "golangci/golangci-lint-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/lint.yml
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
[error] 26-26: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI (ubuntu-latest, 1.23.x)
michael1011
left a comment
michael1011
left a comment
There was a problem hiding this comment.
Also, I cannot get the action to pass
| - name: Lint | ||
| uses: golangci/golangci-lint-action@v3 | ||
| with: | ||
| version: "latest" No newline at end of file |
jackstar12
force-pushed
the
ci/lint-action
branch
from
2860288 to
39d81dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/lint.yml (1)
25-26: Add a newline at the end of the fileYAML files conventionally end with a newline character.
version: v2.0 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
🧹 Nitpick comments (1)
cmd/boltzcli/utils.go (1)
152-156: Consider using less severe error handlingThe current implementation calls
logger.Fatal()which terminates the program if there's an error printing a colored message. This might be too drastic for a UI-related function.Consider using a less severe approach:
func colorPrintln(color *color.Color, message string) { if _, err := color.Println(message); err != nil { - logger.Fatal(err.Error()) + logger.Errorf("Failed to print colored message: %v", err) + fmt.Println(message) // Fallback to regular printing } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.github/workflows/lint.yml(1 hunks).github/workflows/test.yml(0 hunks)cmd/boltzcli/commands.go(12 hunks)cmd/boltzcli/utils.go(2 hunks)internal/autoswap/lightning.go(4 hunks)internal/config/config.go(1 hunks)internal/database/any.go(2 hunks)internal/database/chain.go(5 hunks)internal/database/database.go(1 hunks)internal/database/entity.go(1 hunks)internal/database/macaroons.go(1 hunks)internal/database/migration.go(2 hunks)internal/database/migration_test.go(1 hunks)internal/database/reverse.go(3 hunks)internal/database/swap.go(4 hunks)internal/database/wallet.go(2 hunks)internal/macaroons/permissions.go(1 hunks)internal/mempool/mempool.go(2 hunks)internal/nursery/chain.go(4 hunks)internal/nursery/reverse.go(1 hunks)internal/nursery/swap.go(2 hunks)internal/onchain/boltz.go(1 hunks)internal/onchain/onchain.go(1 hunks)internal/onchain/wallet/wallet.go(3 hunks)internal/onchain/wallet/wallet_test.go(2 hunks)internal/rpcserver/router.go(1 hunks)internal/rpcserver/server.go(2 hunks)internal/test/test.go(3 hunks)internal/utils/version.go(1 hunks)pkg/boltz/btc.go(1 hunks)pkg/boltz/liquid.go(4 hunks)pkg/boltz/swaptree.go(1 hunks)pkg/boltz/transaction.go(2 hunks)pkg/boltz/ws.go(1 hunks)pkg/boltzrpc/client/autoswap.go(3 hunks)pkg/boltzrpc/serializers/utils.go(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
✅ Files skipped from review due to trivial changes (12)
- internal/nursery/reverse.go
- pkg/boltz/btc.go
- internal/database/macaroons.go
- internal/config/config.go
- internal/nursery/swap.go
- internal/utils/version.go
- internal/database/migration.go
- pkg/boltzrpc/client/autoswap.go
- internal/database/swap.go
- internal/database/wallet.go
- pkg/boltz/ws.go
- pkg/boltz/liquid.go
🧰 Additional context used
🧬 Code Graph Analysis (13)
internal/database/database.go (1)
internal/logger/logger.go (1)
Errorf(97-99)
pkg/boltz/transaction.go (2)
internal/database/reverse.go (1)
ReverseSwap(17-48)pkg/boltz/api.go (1)
NormalSwap(27-27)
cmd/boltzcli/utils.go (1)
internal/logger/logger.go (1)
Fatal(84-87)
internal/nursery/chain.go (1)
internal/logger/logger.go (1)
Errorf(97-99)
internal/rpcserver/server.go (1)
pkg/boltz/network.go (3)
MainNet(17-25)TestNet(27-35)Regtest(37-45)
pkg/boltz/swaptree.go (3)
internal/database/reverse.go (1)
ReverseSwap(17-48)internal/database/chain.go (1)
ChainSwap(15-31)pkg/boltz/api.go (1)
NormalSwap(27-27)
internal/onchain/wallet/wallet.go (2)
pkg/boltz/currency.go (2)
CurrencyBtc(11-11)CurrencyLiquid(12-12)pkg/boltz/network.go (4)
Network(10-15)MainNet(17-25)TestNet(27-35)Regtest(37-45)
internal/database/chain.go (1)
internal/logger/logger.go (1)
Errorf(97-99)
internal/onchain/onchain.go (1)
pkg/boltz/currency.go (2)
CurrencyLiquid(12-12)CurrencyBtc(11-11)
cmd/boltzcli/commands.go (2)
pkg/boltzrpc/boltzrpc.pb.go (7)
Budget(4238-4247)Budget(4262-4262)Budget(4277-4279)SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)internal/logger/logger.go (1)
Errorf(97-99)
pkg/boltzrpc/serializers/utils.go (3)
pkg/boltz/api.go (1)
NormalSwap(27-27)pkg/boltzrpc/boltzrpc.pb.go (2)
SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)internal/database/reverse.go (1)
ReverseSwap(17-48)
internal/macaroons/permissions.go (2)
pkg/boltzrpc/boltzrpc.pb.go (2)
MacaroonAction_READ(27-27)MacaroonAction_WRITE(28-28)pkg/boltzrpc/client/client.go (1)
ReadPermissions(25-27)
internal/mempool/mempool.go (1)
internal/logger/logger.go (1)
Errorf(97-99)
🪛 golangci-lint (1.64.8)
internal/onchain/boltz.go
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
🪛 YAMLlint (1.35.1)
.github/workflows/lint.yml
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI (ubuntu-latest, 1.23.x)
🔇 Additional comments (40)
internal/database/database.go (1)
568-572: Good addition for consistent DB resource managementThe
closeRowsfunction centralizes error handling for database row closure, ensuring that any errors are properly logged. This is a solid improvement for consistency across the codebase.internal/onchain/wallet/wallet_test.go (2)
5-9: Import reorganization looks goodMoving standard library packages to the top of the import block follows Go style conventions.
92-92: Simplified access to Inputs fieldThe change from
liquidTx.Transaction.InputstoliquidTx.Inputssimplifies the access pattern. Ensure that this change is consistent with any refactoring of theLiquidTransactionstruct.internal/mempool/mempool.go (2)
63-67: Improved error handling for response body closure.Good enhancement to the error handling mechanism. Using an anonymous function with error checking for
res.Body.Close()ensures that any issues during resource cleanup are properly logged instead of being silently ignored.
129-134: Refactored conditional logic to use a switch statement.Good refactoring of the WebSocket scheme handling. Using a switch statement improves code readability and makes the different cases more explicit compared to the previous if-else structure. This is more maintainable if additional schemes need to be supported in the future.
internal/test/test.go (3)
13-15: Reformatted import statements for better readability.Good organization with each external package import on its own line, separated from internal imports. This improves code readability.
45-47: Enhanced error handling in ClearWalletDataDir function.Good improvement to the function signature by returning the error from
os.RemoveAllrather than silently ignoring it. This follows Go's error handling idioms and allows callers to deal with potential errors.
67-69: Added proper error checking for ClearWalletDataDir.Good addition of error handling for the
ClearWalletDataDircall. This ensures that any errors during directory cleanup are propagated upward, preventing potential issues if the wallet data directory couldn't be properly cleared.pkg/boltzrpc/serializers/utils.go (1)
38-45: Refactored conditional logic to use a switch statement.Good refactoring from if-else chains to a switch statement. This improves code readability and maintainability when handling multiple cases. The added default case also ensures all possible values are properly handled.
internal/nursery/chain.go (1)
46-47: Improved error wrapping using fmt.Errorf with %w verb.Good enhancement to the error handling by using
fmt.Errorfwith the%wverb instead of simple string concatenation. This properly wraps the original error while preserving its type information, allowing for better error inspection and unwrapping by callers.Also applies to: 55-56, 92-93, 96-97
internal/onchain/wallet/wallet.go (3)
26-28: Import organization looks good.The reorganization of the import statements improves code structure by grouping related imports together.
237-242: Good refactoring from if-else to switch statement.Converting the conditional logic from nested if-statements to a switch statement improves readability and makes the code structure more maintainable. The switch statement is more appropriate here since you're checking discrete values of
handlerStatus.Status.
348-375: Well-structured nested switch statements.The conversion from if-else chains to switch statements in this section significantly improves code readability. The nested structure clearly separates the logic for different currencies and networks, making it easier to understand and maintain.
pkg/boltz/swaptree.go (1)
188-203: Good refactoring of conditional logic.Converting the if-else chain to a switch statement improves code readability. I also notice that you moved the
claim.AddOp(txscript.OP_CHECKSIG)operation outside the switch statement, which is correct as it should be applied regardless of the swap type.The switch statement also makes it more explicit that
ReverseSwapandChainSwapshare the same behavior, which was less obvious in the if-else structure.internal/database/migration_test.go (1)
31-31: Improved error handling for database close operation.Adding error checking with
require.NoError(t, db.Close())is a good practice. This ensures that any errors during connection closure are properly caught and will cause the test to fail appropriately.internal/database/entity.go (1)
56-56: Consistent resource cleanup improvement.Replacing
rows.Close()withcloseRows(rows)enhances error handling during row cleanup. This is a good practice to ensure any errors during row closure are properly logged or handled, and it maintains consistency with the rest of the codebase.internal/macaroons/permissions.go (1)
243-259: Refactoring to switch statement improves code clarityThe change from if-else to switch statement for checking permission actions enhances readability and makes the control flow more explicit. This is a good refactoring that maintains the existing logic while making the code more maintainable.
internal/database/chain.go (5)
8-9: Adding time package importThe addition of the time import is appropriate, as it's used in the file (e.g., for the CreatedAt field in the ChainSwap struct).
259-259: Improved resource management with closeRowsReplacing direct
rows.Close()withcloseRows(rows)enhances error handling during database resources cleanup.
281-281: Consistent use of closeRows functionConsistent application of the
closeRows(rows)pattern throughout the codebase improves error handling and resource management.
290-291: Better error formatting with fmt.ErrorfUsing
fmt.Errorfinstead of string concatenation is a better practice for error message construction, improving consistency with the rest of the codebase.
421-422: Consistent cleanup pattern appliedThe pattern of using
closeRows(rows)is consistently applied throughout the file, which is good for maintainability.internal/database/any.go (2)
80-81: Consistent resource cleanup with closeRowsThe change from
rows.Close()tocloseRows(rows)aligns with the improved error handling pattern used throughout the database package.
97-98: Consistent application of closeRows patternConsistently applying the
closeRows(rows)pattern across all database operations ensures uniform error handling during resource cleanup.pkg/boltz/transaction.go (2)
170-201: Improved code readability with switch statementThe refactoring from if-else to switch statement is a good change that improves code readability and maintainability when handling different swap types. The switch structure makes the control flow more explicit and easier to follow.
6-7: Clean import organizationThe import reorganization with math package on its own line improves readability.
internal/onchain/onchain.go (1)
206-211: Improved currency handling with switch statementThe refactoring to a switch statement based directly on the currency parameter is a cleaner approach. It eliminates an unnecessary level of indirection by checking the currency directly rather than deriving it from the chain variable, making the code more maintainable.
internal/database/reverse.go (3)
226-226: Enhanced resource managementReplacing
defer rows.Close()withdefer closeRows(rows)is a good practice for improved error handling. This change ensures that any errors during closing of database resources are properly handled.
249-249: Enhanced resource managementConsistent use of
closeRows(rows)instead of directrows.Close()call improves error handling throughout the codebase.
290-290: Enhanced resource managementThe consistent application of the
closeRows(rows)pattern ensures uniform resource cleanup across the database package.internal/rpcserver/server.go (2)
18-23: Import statement reorganizationThe import statements have been reorganized in a more structured manner, separating the internal packages from external ones.
416-426: Improved network configuration with switch statementThe refactoring to a switch statement for network-based configuration improves clarity and maintainability. Additionally, the changes:
- Add specific regtest environment configuration
- Update the TestNet Liquid API endpoint to use bullbitcoin.com instead of liquid.network
This is a good improvement that makes the code more structured and handles all network types explicitly.
cmd/boltzcli/commands.go (4)
17-20: Import organization improves code structure.The imports have been neatly organized, with related packages grouped together, improving readability.
70-70: Good refactoring to centralize color printing logic.Replacing direct calls to color printing methods with the new
colorPrintlnfunction improves consistency and error handling throughout the codebase. This change follows the DRY principle and makes future modifications to the output formatting easier to implement.Also applies to: 289-289, 291-291, 314-314, 316-316, 344-344, 346-346, 558-558, 560-560, 563-563, 570-570, 580-580
2132-2136: Improved error handling for file operations.The updated defer statement now properly logs any errors that occur when closing the file. This is a good practice that helps identify potential file system issues that might otherwise be silently ignored.
703-712: Better code structure with switch statements.Replacing if-else chains with switch statements improves readability and maintainability. This change makes the code more idiomatic Go and will make it easier to add new cases in the future if needed.
Also applies to: 860-865, 1407-1412, 1824-1830
internal/autoswap/lightning.go (3)
6-9: Consistent import organization.The imports have been organized in a consistent manner with the other files in the codebase, improving overall readability.
167-167: Simplified property access.Directly accessing
cfg.Currencyinstead ofcfg.SerializedLnConfig.Currencysimplifies the code by removing an unnecessary level of indirection.
401-419: Improved control flow with switch statement.Replacing the if-else structure with a switch statement for handling different swap types makes the code more maintainable and easier to extend in the future. This is consistent with the changes made in other files.
internal/rpcserver/router.go (1)
292-301: Improved request type handling with switch statement.Replacing the if-else structure with a switch statement for handling different request types makes the code more readable and maintainable. The addition of a default case with a specific error message ("unknown swap type") also improves error reporting compared to the previous generic error.
| - name: Install Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: 1.23.x |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the Go version specified
The workflow specifies Go version 1.23.x, which might be a typo or incorrect since the latest stable version as of April 2024 is 1.22.x.
🌐 Web query:
What is the latest stable version of Go?
💡 Result:
The latest stable version of Go is 1.24.2, released on April 1, 2025[1][3][7]. This is a minor revision of the Go 1.24 major release, which was first introduced on February 11, 2025[1][5]. The update includes security fixes for the net/http package and bug fixes across several components like the compiler, runtime, and crypto/tls package[1][7].
Key Details:
- Support Policy: Go follows a support window where each major release is maintained until two newer major versions exist. As of April 2025:
- Go 1.24 is actively supported[1][3].
- Go 1.23 (released August 13, 2024) is also supported but will be phased out once Go 1.25 is released[1][3].
- Older versions like Go 1.22 and earlier are no longer supported[3].
Notable Features in Go 1.24:
- Enhanced FIPS 140-3 compliance with a new cryptographic module[5].
- Experimental
testing/synctestpackage for concurrent code testing[5]. - WebAssembly improvements, including reduced memory usage and reactor/library build support[5].
For most users, upgrading to Go 1.24.2 is recommended to benefit from the latest security patches and optimizations.
Citations:
- 1: https://go.dev/doc/devel/release
- 2: https://github.com/golang/tools/releases
- 3: https://endoflife.date/go
- 4: https://en.wikipedia.org/wiki/Go_(programming_language)
- 5: https://tip.golang.org/doc/go1.24
- 6: https://tip.golang.org/doc/go1.23
- 7: https://go.dev/dl/
- 8: https://groups.google.com/g/golang-nuts/c/_ROx_UGCCCg
Action Required: Update Go Version in Workflow
The workflow currently specifies the Go version as:
go-version: 1.23.xHowever, our latest verification shows that the current stable Go release is 1.24.2 (released on April 1, 2025). While Go 1.23 remains supported for now, updating to 1.24.2 is recommended to benefit from recent security fixes, performance improvements, and bug fixes.
- Location:
.github/workflows/lint.ymlat line 21 - Suggestion: Replace
1.23.xwith1.24.2to align with the latest stable version.
jackstar12
force-pushed
the
ci/lint-action
branch
from
39d81dc to
f6a7981
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
internal/lightning/lib/bolt12/target/bolt12.h (1)
1-33: 🛠️ Refactor suggestionAdd header guards to prevent multiple inclusion.
This header file is missing the standard include guards or
#pragma oncedirective to prevent multiple inclusion, which could lead to compilation errors.+#ifndef BOLT12_H +#define BOLT12_H + #include <stdarg.h> #include <stdbool.h> #include <stdint.h> #include <stdlib.h> // ... rest of the file ... void free_c_string(char *s); + +#endif /* BOLT12_H */
♻️ Duplicate comments (1)
internal/onchain/boltz.go (1)
24-25:⚠️ Potential issueFix method call -
GetTransactionDetailsis undefinedThe
BoltzTxProviderstruct doesn't have aGetTransactionDetailsmethod defined, but the code is attempting to call it. This will cause a compilation error as flagged by the static analysis tool.Apply this fix:
- transaction, err := txProvider.GetTransactionDetails(txId, txProvider.currency) + transaction, err := txProvider.Api.GetTransactionDetails(txId, txProvider.currency)🧰 Tools
🪛 golangci-lint (1.64.8)
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
🧹 Nitpick comments (2)
internal/lightning/lib/bolt12/target/bolt12.h (2)
1-4: Review unused header include.The header file includes
stdarg.hwhich doesn't appear to be used by any of the function declarations in this file. Consider removing unused includes to avoid potential confusion about dependencies.-#include <stdarg.h> #include <stdbool.h> #include <stdint.h> #include <stdlib.h>
6-8: Consider expanding the Offer structure for complete BOLT12 support.The current
Offerstructure only contains a single field (min_amount_sat), but the BOLT12 specification includes many more fields such as description, issuer, and other metadata. This implementation appears to be simplified, which might be intentional for your specific use case, but could limit future functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
cmd/boltzcli/commands.go(12 hunks)cmd/boltzcli/utils.go(2 hunks)internal/autoswap/lightning.go(4 hunks)internal/config/config.go(1 hunks)internal/database/any.go(2 hunks)internal/database/chain.go(5 hunks)internal/database/database.go(1 hunks)internal/database/entity.go(1 hunks)internal/database/macaroons.go(1 hunks)internal/database/migration.go(2 hunks)internal/database/migration_test.go(1 hunks)internal/database/reverse.go(3 hunks)internal/database/swap.go(4 hunks)internal/database/wallet.go(2 hunks)internal/lightning/lib/bolt12/target/bolt12.h(1 hunks)internal/macaroons/permissions.go(1 hunks)internal/mempool/mempool.go(2 hunks)internal/nursery/chain.go(4 hunks)internal/nursery/reverse.go(1 hunks)internal/nursery/swap.go(2 hunks)internal/onchain/boltz.go(1 hunks)internal/onchain/onchain.go(1 hunks)internal/onchain/wallet/wallet.go(3 hunks)internal/onchain/wallet/wallet_test.go(2 hunks)internal/rpcserver/router.go(1 hunks)internal/rpcserver/server.go(2 hunks)internal/test/test.go(3 hunks)internal/utils/version.go(1 hunks)pkg/boltz/btc.go(1 hunks)pkg/boltz/liquid.go(4 hunks)pkg/boltz/swaptree.go(2 hunks)pkg/boltz/transaction.go(2 hunks)pkg/boltz/ws.go(1 hunks)pkg/boltzrpc/client/autoswap.go(3 hunks)pkg/boltzrpc/serializers/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (31)
- pkg/boltz/btc.go
- internal/config/config.go
- internal/database/database.go
- internal/database/macaroons.go
- internal/database/any.go
- internal/nursery/reverse.go
- internal/database/wallet.go
- internal/database/entity.go
- pkg/boltz/liquid.go
- internal/database/migration_test.go
- internal/nursery/chain.go
- pkg/boltz/ws.go
- internal/mempool/mempool.go
- internal/database/swap.go
- internal/database/migration.go
- internal/onchain/wallet/wallet_test.go
- pkg/boltzrpc/serializers/utils.go
- pkg/boltz/swaptree.go
- internal/utils/version.go
- internal/nursery/swap.go
- internal/macaroons/permissions.go
- pkg/boltz/transaction.go
- internal/onchain/onchain.go
- cmd/boltzcli/commands.go
- internal/database/chain.go
- internal/rpcserver/server.go
- internal/onchain/wallet/wallet.go
- pkg/boltzrpc/client/autoswap.go
- internal/database/reverse.go
- internal/autoswap/lightning.go
- internal/rpcserver/router.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/boltzcli/utils.go (1)
internal/logger/logger.go (1)
Fatal(84-87)
🪛 golangci-lint (1.64.8)
internal/onchain/boltz.go
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI (ubuntu-latest, 1.23.x)
🔇 Additional comments (8)
cmd/boltzcli/utils.go (2)
11-17: LGTM: Import organization looks goodThe imports have been reorganized to include the required packages for the new functionality.
152-156: New utility function implementation looks goodThe
colorPrintlnfunction correctly handles color printing with proper error handling usinglogger.Fatal. This centralization of color printing logic will improve code maintainability across the codebase.internal/test/test.go (3)
13-15: Approve the import reorganization.The imports have been reorganized to explicitly list
github.com/BoltzExchange/go-electrum/electrumandgolang.org/x/sync/errgroup, which improves code clarity.
45-46: Good improvement in error handling.The function signature has been updated to return an error, which is a good practice in Go as it properly propagates errors up the call stack instead of ignoring them. This change allows callers to be aware of and handle any issues that might occur during directory removal.
67-69: Proper error handling implementation.This change correctly implements the handling of potential errors from
ClearWalletDataDir(). By checking for errors and returning early if one occurs, the function ensures that any issues during wallet data directory cleanup are properly reported to the caller rather than being silently ignored.internal/lightning/lib/bolt12/target/bolt12.h (3)
10-13: LGTM - Good error handling pattern.The
CResult_Offerstructure follows a good pattern for error handling in C, allowing functions to return either a successful result or an error message. The use ofconstfor both fields is appropriate to prevent modification of the returned data.
15-19: LGTM - Invoice structure with appropriate field types.The
Invoicestructure contains essential fields with appropriate types - usinguint64_tfor amounts and timestamps, and a fixed-size array for the payment hash. The 32-byte size forpayment_hashcorrectly matches the SHA-256 hash size used in Lightning Network.
21-24: LGTM - Consistent error handling pattern.The
CResult_Invoicestructure follows the same error handling pattern asCResult_Offer, maintaining consistency in the API design.
jackstar12
force-pushed
the
ci/lint-action
branch
from
f6a7981 to
97756ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
internal/onchain/boltz.go (1)
25-25:⚠️ Potential issueFix method call -
GetTransactionDetailsis undefinedThe
BoltzTxProviderstruct doesn't have aGetTransactionDetailsmethod defined, but the code is attempting to call it. This will cause a compilation error as flagged by the static analysis tool.Apply this fix:
- transaction, err := txProvider.GetTransactionDetails(txId, txProvider.currency) + transaction, err := txProvider.Api.GetTransactionDetails(txId, txProvider.currency)🧰 Tools
🪛 golangci-lint (1.64.8)
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
internal/lightning/lib/bolt12/target/bolt12.h (1)
26-32: Add documentation and resource management functions.The function declarations lack documentation about their behavior, error conditions, and memory ownership. Additionally, while there is a
free_c_stringfunction, there are no corresponding functions to free theOfferandInvoicestructures returned by the decode functions.Consider adding:
- Function documentation comments
- Free functions for the result structures
+/** + * Decodes a BOLT12 offer string. + * @param offer The BOLT12 offer string to decode. + * @return A CResult_Offer containing either the decoded Offer or an error message. + * The caller is responsible for freeing the returned Offer using free_offer(). + */ struct CResult_Offer decode_offer(const char *offer); +/** + * Decodes a BOLT12 invoice string. + * @param invoice The BOLT12 invoice string to decode. + * @return A CResult_Invoice containing either the decoded Invoice or an error message. + * The caller is responsible for freeing the returned Invoice using free_invoice(). + */ struct CResult_Invoice decode_invoice(const char *invoice); +/** + * Checks if an invoice corresponds to a specific offer. + * @param invoice The BOLT12 invoice string to check. + * @param offer The BOLT12 offer string to check against. + * @return true if the invoice is for the offer, false otherwise. + */ bool check_invoice_is_for_offer(const char *invoice, const char *offer); +/** + * Frees memory allocated for a C string. + * @param s The string to free. + */ void free_c_string(char *s); + +/** + * Frees memory allocated for an Offer structure. + * @param offer The Offer structure to free. + */ +void free_offer(struct Offer *offer); + +/** + * Frees memory allocated for an Invoice structure. + * @param invoice The Invoice structure to free. + */ +void free_invoice(struct Invoice *invoice);.github/workflows/lint.yml (2)
26-26: Ensure the file ends with a newline.
YAML files should end with a newline character to avoid potential parsing issues. This issue was previously noted.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
21-21: 💡 Verification agent❓ Verification inconclusive
Update Go version to the latest stable release.
The workflow currently specifiesgo-version: 1.23.x; however, to benefit from the newest security patches, bug fixes, and performance improvements, please update it to1.24.2.
Update Go Version in CI Workflow
Please update the Go version in your GitHub Actions workflow by replacing the outdated line:
- go-version: 1.23.x + go-version: 1.24.2This change will ensure you benefit from the latest security patches, bug fixes, and performance improvements. Make sure to update line 21 in
.github/workflows/lint.yml.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.github/workflows/lint.yml(1 hunks).github/workflows/test.yml(0 hunks)cmd/boltzcli/commands.go(12 hunks)cmd/boltzcli/utils.go(2 hunks)internal/autoswap/lightning.go(4 hunks)internal/config/config.go(1 hunks)internal/database/any.go(2 hunks)internal/database/chain.go(4 hunks)internal/database/database.go(1 hunks)internal/database/entity.go(1 hunks)internal/database/macaroons.go(1 hunks)internal/database/migration.go(2 hunks)internal/database/migration_test.go(1 hunks)internal/database/reverse.go(3 hunks)internal/database/swap.go(4 hunks)internal/database/wallet.go(2 hunks)internal/lightning/lib/bolt12/target/bolt12.h(1 hunks)internal/macaroons/permissions.go(1 hunks)internal/mempool/mempool.go(2 hunks)internal/nursery/chain.go(3 hunks)internal/nursery/reverse.go(1 hunks)internal/nursery/swap.go(1 hunks)internal/onchain/boltz.go(1 hunks)internal/onchain/onchain.go(1 hunks)internal/onchain/wallet/wallet.go(3 hunks)internal/onchain/wallet/wallet_test.go(2 hunks)internal/rpcserver/router.go(1 hunks)internal/rpcserver/server.go(2 hunks)internal/test/test.go(3 hunks)internal/utils/version.go(1 hunks)pkg/boltz/btc.go(1 hunks)pkg/boltz/liquid.go(4 hunks)pkg/boltz/swaptree.go(2 hunks)pkg/boltz/transaction.go(2 hunks)pkg/boltz/ws.go(1 hunks)pkg/boltzrpc/client/autoswap.go(3 hunks)pkg/boltzrpc/serializers/utils.go(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
🚧 Files skipped from review as they are similar to previous changes (30)
- internal/nursery/swap.go
- internal/database/wallet.go
- internal/database/migration_test.go
- internal/database/database.go
- internal/database/macaroons.go
- internal/nursery/reverse.go
- pkg/boltz/ws.go
- internal/nursery/chain.go
- pkg/boltzrpc/client/autoswap.go
- internal/database/migration.go
- internal/onchain/wallet/wallet.go
- pkg/boltzrpc/serializers/utils.go
- internal/database/chain.go
- pkg/boltz/liquid.go
- pkg/boltz/swaptree.go
- pkg/boltz/transaction.go
- internal/database/swap.go
- internal/onchain/wallet/wallet_test.go
- internal/macaroons/permissions.go
- internal/mempool/mempool.go
- internal/database/any.go
- internal/database/reverse.go
- internal/onchain/onchain.go
- cmd/boltzcli/commands.go
- internal/autoswap/lightning.go
- internal/config/config.go
- pkg/boltz/btc.go
- internal/database/entity.go
- internal/utils/version.go
- internal/rpcserver/router.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/rpcserver/server.go (1)
pkg/boltz/network.go (3)
MainNet(17-25)TestNet(27-35)Regtest(37-45)
🪛 YAMLlint (1.35.1)
.github/workflows/lint.yml
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
🪛 golangci-lint (1.64.8)
internal/onchain/boltz.go
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI (ubuntu-latest, 1.23.x)
🔇 Additional comments (7)
cmd/boltzcli/utils.go (2)
11-15: Reordering of imports looks goodThe imports have been reorganized to group related packages together, which follows good Go coding practices.
151-155: Well-implemented error handling in the new colorPrintln functionThe new
colorPrintlnfunction encapsulates color printing functionality with proper error handling usinglogger.Fatal. This is a good abstraction that centralizes the error handling logic for color printing operations.internal/rpcserver/server.go (2)
18-22: Improved import organization.The imports have been reorganized to group related packages together, enhancing code readability. This is a good practice for maintaining clean and organized code.
416-426: Good refactoring of network type handling.Replacing the if-else structure with a switch statement improves code readability and maintainability. The switch statement explicitly handles all three network types (MainNet, TestNet, Regtest) and properly sets the appropriate API endpoints for each environment.
The addition of explicit Regtest handling with localhost endpoints is particularly helpful for local development and testing scenarios.
internal/test/test.go (3)
13-15: Clean import formattingThe imports have been reorganized to follow Go's conventional import structure: standard library packages, followed by external dependencies, and then internal packages. This is a good practice that improves code readability.
45-46: Improved error handling in ClearWalletDataDirGood improvement to return the error from
os.RemoveAll()instead of silently ignoring it. This change allows callers to properly handle potential file system errors.
67-69: Proper error handling implementationThe function now correctly checks for errors from
ClearWalletDataDir()and returns early if an error occurs. This follows Go's idiomatic error handling pattern and prevents executing subsequent code when the directory cleanup fails.
jackstar12
force-pushed
the
ci/lint-action
branch
from
97756ae to
aba391c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
internal/onchain/boltz.go (1)
24-25:⚠️ Potential issueFix method call -
GetTransactionDetailsis undefinedThe
BoltzTxProviderstruct doesn't have aGetTransactionDetailsmethod defined, but the code is attempting to call it directly. This will cause a compilation error as flagged by the static analysis tool.Apply this fix:
- transaction, err := txProvider.GetTransactionDetails(txId, txProvider.currency) + transaction, err := txProvider.Api.GetTransactionDetails(txId, txProvider.currency)🧰 Tools
🪛 golangci-lint (1.64.8)
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
internal/lightning/lib/bolt12/target/bolt12.h (5)
6-8: Add documentation for the Offer structure.The
Offerstructure lacks documentation explaining what it represents and the purpose of themin_amount_satfield. Consider adding comments to make this API more maintainable and easier to understand for other developers.+/** + * Represents a BOLT12 offer with payment information. + */ typedef struct Offer { + /** + * Minimum amount in satoshis that can be paid with this offer. + */ uint64_t min_amount_sat; } Offer;
10-13: Document the result/error pattern for CResult structures.The
CResult_Offerstructure implements a common result/error pattern but lacks documentation explaining how to interpret the result. A user needs to know that only one of the fields will be non-NULL at a time.+/** + * Result wrapper for Offer operations. + * Only one of result or error will be non-NULL. + * If result is non-NULL, the operation was successful. + * If error is non-NULL, the operation failed and error contains the error message. + */ typedef struct CResult_Offer { const struct Offer *result; const char *error; } CResult_Offer;
15-19: Add documentation for the Invoice structure.The
Invoicestructure lacks documentation explaining its purpose and fields. Consider adding comments to clarify what each field represents, particularly the format ofpayment_hashand the time unit forexpiry_date.+/** + * Represents a BOLT12 invoice for payment. + */ typedef struct Invoice { + /** + * Amount to be paid in satoshis. + */ uint64_t amount_sat; + /** + * SHA-256 hash of the payment preimage. + */ uint8_t payment_hash[32]; + /** + * Expiry date as UNIX timestamp in seconds. + */ uint64_t expiry_date; } Invoice;
21-24: Document the CResult_Invoice structure.Similar to the
CResult_Offerstructure,CResult_Invoicelacks documentation explaining the result/error pattern.+/** + * Result wrapper for Invoice operations. + * Only one of result or error will be non-NULL. + * If result is non-NULL, the operation was successful. + * If error is non-NULL, the operation failed and error contains the error message. + */ typedef struct CResult_Invoice { const struct Invoice *result; const char *error; } CResult_Invoice;
26-32: Add documentation and resource management functions.The function declarations lack documentation about their behavior, error conditions, and memory ownership. Additionally, while there is a
free_c_stringfunction, there are no corresponding functions to free theOfferandInvoicestructures returned by the decode functions.Consider adding:
- Function documentation comments
- Free functions for the result structures
+/** + * Decodes a BOLT12 offer string into an Offer structure. + * + * @param offer The BOLT12 offer string to decode + * @return A CResult_Offer containing either the decoded Offer or an error message + */ struct CResult_Offer decode_offer(const char *offer); + +/** + * Decodes a BOLT12 invoice string into an Invoice structure. + * + * @param invoice The BOLT12 invoice string to decode + * @return A CResult_Invoice containing either the decoded Invoice or an error message + */ struct CResult_Invoice decode_invoice(const char *invoice); + +/** + * Checks if an invoice corresponds to a specific offer. + * + * @param invoice The BOLT12 invoice string + * @param offer The BOLT12 offer string + * @return true if the invoice is for the offer, false otherwise + */ bool check_invoice_is_for_offer(const char *invoice, const char *offer); + +/** + * Frees memory allocated for a C string. + * + * @param s The string to free + */ void free_c_string(char *s); + +/** + * Frees memory allocated for an Offer structure. + * + * @param offer The Offer structure to free + */ +void free_offer(struct Offer *offer); + +/** + * Frees memory allocated for an Invoice structure. + * + * @param invoice The Invoice structure to free + */ +void free_invoice(struct Invoice *invoice);
🧹 Nitpick comments (1)
internal/lightning/lib/bolt12/target/bolt12.h (1)
1-33: Add version information and copyright notice.The header file lacks version information and copyright notices, which are common in library APIs to help with versioning and legal compliance.
+/** + * @file bolt12.h + * @brief BOLT12 offer and invoice handling API + * @version 0.1.0 + * @copyright Copyright (c) 2023 [Your Organization] + * + * This file provides structures and functions for working with BOLT12 + * offers and invoices in the Lightning Network. + */ + #include <stdarg.h> #include <stdbool.h> #include <stdint.h> #include <stdlib.h>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
cmd/boltzcli/commands.go(12 hunks)cmd/boltzcli/utils.go(2 hunks)internal/autoswap/lightning.go(4 hunks)internal/config/config.go(1 hunks)internal/database/any.go(2 hunks)internal/database/chain.go(4 hunks)internal/database/database.go(1 hunks)internal/database/entity.go(1 hunks)internal/database/macaroons.go(1 hunks)internal/database/migration.go(2 hunks)internal/database/migration_test.go(1 hunks)internal/database/reverse.go(3 hunks)internal/database/swap.go(4 hunks)internal/database/wallet.go(2 hunks)internal/lightning/lib/bolt12/target/bolt12.h(1 hunks)internal/macaroons/permissions.go(1 hunks)internal/mempool/mempool.go(2 hunks)internal/nursery/chain.go(3 hunks)internal/nursery/reverse.go(1 hunks)internal/nursery/swap.go(1 hunks)internal/onchain/boltz.go(1 hunks)internal/onchain/onchain.go(1 hunks)internal/onchain/wallet/wallet.go(3 hunks)internal/onchain/wallet/wallet_test.go(2 hunks)internal/rpcserver/router.go(1 hunks)internal/rpcserver/server.go(2 hunks)internal/test/test.go(3 hunks)internal/utils/version.go(1 hunks)pkg/boltz/btc.go(1 hunks)pkg/boltz/liquid.go(4 hunks)pkg/boltz/swaptree.go(2 hunks)pkg/boltz/transaction.go(2 hunks)pkg/boltz/ws.go(1 hunks)pkg/boltzrpc/client/autoswap.go(3 hunks)pkg/boltzrpc/serializers/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (33)
- internal/config/config.go
- pkg/boltz/btc.go
- internal/database/macaroons.go
- internal/database/database.go
- internal/database/entity.go
- internal/nursery/reverse.go
- internal/database/migration_test.go
- internal/nursery/swap.go
- internal/macaroons/permissions.go
- internal/database/any.go
- internal/database/chain.go
- internal/database/wallet.go
- pkg/boltz/liquid.go
- internal/database/migration.go
- internal/onchain/wallet/wallet_test.go
- internal/mempool/mempool.go
- pkg/boltz/swaptree.go
- internal/onchain/wallet/wallet.go
- internal/database/reverse.go
- pkg/boltzrpc/client/autoswap.go
- cmd/boltzcli/utils.go
- pkg/boltzrpc/serializers/utils.go
- internal/nursery/chain.go
- pkg/boltz/transaction.go
- internal/onchain/onchain.go
- internal/test/test.go
- internal/rpcserver/server.go
- internal/autoswap/lightning.go
- pkg/boltz/ws.go
- internal/database/swap.go
- cmd/boltzcli/commands.go
- internal/utils/version.go
- internal/rpcserver/router.go
🧰 Additional context used
🪛 golangci-lint (1.64.8)
internal/onchain/boltz.go
25-25: txProvider.GetTransactionDetails undefined (type BoltzTxProvider has no field or method GetTransactionDetails)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI (ubuntu-latest, 1.23.x)
| #include <stdarg.h> | ||
| #include <stdbool.h> | ||
| #include <stdint.h> | ||
| #include <stdlib.h> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding a header guard and API visibility macros.
This header file lacks both a header guard to prevent multiple inclusion and API visibility macros to control symbol export. These are important for ensuring correct behavior when the library is included in different contexts.
+#ifndef BOLT12_H
+#define BOLT12_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>
+
+/* Define API visibility macros if not already defined */
+#ifndef BOLT12_API
+ #if defined(_WIN32) || defined(__CYGWIN__)
+ #ifdef BOLT12_BUILD_SHARED
+ #define BOLT12_API __declspec(dllexport)
+ #else
+ #define BOLT12_API __declspec(dllimport)
+ #endif
+ #else
+ #if __GNUC__ >= 4
+ #define BOLT12_API __attribute__((visibility("default")))
+ #else
+ #define BOLT12_API
+ #endif
+ #endif
+#endifAnd at the end of the file:
void free_c_string(char *s);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* BOLT12_H */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <stdarg.h> | |
| #include <stdbool.h> | |
| #include <stdint.h> | |
| #include <stdlib.h> | |
| #ifndef BOLT12_H | |
| #define BOLT12_H | |
| #ifdef __cplusplus | |
| extern "C" { | |
| #endif | |
| #include <stdarg.h> | |
| #include <stdbool.h> | |
| #include <stdint.h> | |
| #include <stdlib.h> | |
| /* Define API visibility macros if not already defined */ | |
| #ifndef BOLT12_API | |
| #if defined(_WIN32) || defined(__CYGWIN__) | |
| #ifdef BOLT12_BUILD_SHARED | |
| #define BOLT12_API __declspec(dllexport) | |
| #else | |
| #define BOLT12_API __declspec(dllimport) | |
| #endif | |
| #else | |
| #if __GNUC__ >= 4 | |
| #define BOLT12_API __attribute__((visibility("default"))) | |
| #else | |
| #define BOLT12_API | |
| #endif | |
| #endif | |
| #endif | |
| void free_c_string(char *s); | |
| #ifdef __cplusplus | |
| } /* extern "C" */ | |
| #endif | |
| #endif /* BOLT12_H */ |
jackstar12
force-pushed
the
ci/lint-action
branch
from
aba391c to
ff0dd32
Compare
2 participants
Summary by CodeRabbit