-
Notifications
You must be signed in to change notification settings - Fork 39
Final refactor #657
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
Final refactor #657
Conversation
WalkthroughThis change is a comprehensive refactor and expansion of the project, rebranding the repository from Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant ArkdServer
participant AdminService
participant RepoManager
participant WalletService
CLI->>ArkdServer: HTTP request (e.g., create intent, get round info)
ArkdServer->>AdminService: Call corresponding method (e.g., CreateNotes, GetRoundDetails)
AdminService->>RepoManager: Query or update database as needed
AdminService->>WalletService: Perform wallet-related operations (e.g., address, balance)
AdminService-->>ArkdServer: Return result
ArkdServer-->>CLI: HTTP response (JSON)
sequenceDiagram
participant ArkdServer
participant Service
participant RepoManager
participant WalletService
participant EventChannel
ArkdServer->>Service: RegisterIntent(proof, message)
Service->>RepoManager: Validate and store intent
Service->>WalletService: Verify BIP322 proof
Service-->>EventChannel: Emit event (IntentRegistered)
Service-->>ArkdServer: Return intent ID
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Shall we drop server, client, SDK and move common to pkg ? So we have flat structure |
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: 8
♻️ Duplicate comments (4)
.github/workflows/integration.yaml (1)
19-20
: Unreleased Go version & trailing whitespace still present
'>=1.23.1'
targets a Go version that does not yet exist, and there is leftover trailing space on the same line. CI will break on the first and linters on the second.- go-version: '>=1.23.1'␠ + # 1.22.x is the latest stable; alternatively use 'stable' + go-version: '1.22.x'.github/workflows/unit.yaml (1)
28-29
: Unreleased Go versionSame issue as in the integration workflow – Go 1.23.x is not released yet.
- go-version: '>=1.23.1' + go-version: '1.22.x'README.md (1)
4-5
: Docker badge still points to/ark
instead of/arkd
The badge URL and hyperlink haven’t been updated to the new image path, so the badge is broken.
-[](https://github.com/arkade-os/arkd/pkgs/container/ark) +[](https://github.com/arkade-os/arkd/pkgs/container/arkd)internal/infrastructure/db/postgres/round_repo.go (1)
614-637
: Don't ignore JSON marshaling errors in createUpsertTransactionParamsThe function silently ignores errors when marshaling the children map, which could hide actual problems and make debugging difficult.
func createUpsertTransactionParams( node tree.TxTreeNode, roundID string, txType string, position int64, -) queries.UpsertTxParams { +) (queries.UpsertTxParams, error) { params := queries.UpsertTxParams{ Tx: node.Tx, RoundID: roundID, Type: txType, Position: int32(position), } if txType == "connector" || txType == "tree" { params.Txid = node.Txid if len(node.Children) > 0 { - // nolint - children, _ := json.Marshal(node.Children) + children, err := json.Marshal(node.Children) + if err != nil { + return queries.UpsertTxParams{}, fmt.Errorf("failed to marshal children: %w", err) + } params.Children = pqtype.NullRawMessage{ RawMessage: children, Valid: true, } } } - return params + return params, nil }Note: This change will require updating all call sites to handle the returned error.
🧹 Nitpick comments (8)
SECURITY.md (1)
16-16
: Consider publishing the full 40-character fingerprint for stronger verificationAll existing rows show only the 16-char short ID. That works, but the modern recommendation is to list the full 40-hex-digit fingerprint so users can validate the correct key with
gpg --with-fingerprint
.
If you have the full value handy, adding it here would improve trustworthiness of the table..github/workflows/integration.yaml (1)
38-38
: Missing newline at EOFPOSIX tools and several linters complain when a newline is absent at the end of the file. Add one for consistency.
internal/infrastructure/db/postgres/sqlc.yaml (1)
38-45
: Add terminating newline for YAML-lint complianceThe file lacks a final newline (YAMLlint error
new-line-at-end-of-file
).- - column: "intent.message" - go_type: "database/sql.NullString" \ No newline at end of file + - column: "intent.message" + go_type: "database/sql.NullString" +.github/workflows/unit.yaml (2)
42-47
: Checkout should precede tool setupBest practice is to
actions/checkout
beforeactions/setup-go
so the latter can cache modules based ongo.sum
. Swapping these two steps saves cache misses.
60-60
: Missing newline at EOFAdd a trailing newline for linter hygiene.
README.md (1)
72-73
: Spelling: “Maximume”Minor typo – should be “Maximum”.
-| `ARKD_REDIS_NUM_OF_RETRIES` | Maximume number of retries for redis write operations in case of conflicts | - | +| `ARKD_REDIS_NUM_OF_RETRIES` | Maximum number of retries for redis write operations in case of conflicts | - |cmd/arkd/utils.go (1)
212-257
: Consider error handling for invalid JSON structureThe generic
post
function assumes the response is always a map with the specified key. If the API returns a different JSON structure whenkey
is not empty, this could cause runtime panics.Consider adding validation:
if key == "" { return } res := make(map[string]T) if err = json.Unmarshal(buf, &res); err != nil { + // Try unmarshaling directly if map unmarshaling fails + var directResult T + if directErr := json.Unmarshal(buf, &directResult); directErr == nil { + result = directResult + return + } return } + if _, exists := res[key]; !exists { + err = fmt.Errorf("key '%s' not found in response", key) + return + } result = res[key]internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (1)
59-105
: Consider adding pagination to prevent memory issues with large datasets.The
SelectAllVtxos
query retrieves all records without any limit. For production systems with many VTXOs, this could cause memory exhaustion.Consider implementing:
- LIMIT/OFFSET based pagination
- Cursor-based pagination for better performance
- A streaming approach for bulk operations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api-spec/protobuf/gen/ark/v1/admin.pb.go
is excluded by!**/*.pb.go
,!**/gen/**
api-spec/protobuf/gen/ark/v1/admin.pb.gw.go
is excluded by!**/*.pb.gw.go
,!**/gen/**
📒 Files selected for processing (28)
.github/workflows/integration.yaml
(2 hunks).github/workflows/unit.yaml
(1 hunks)README.md
(7 hunks)SECURITY.md
(1 hunks)api-spec/openapi/swagger/ark/v1/admin.swagger.json
(4 hunks)api-spec/protobuf/ark/v1/admin.proto
(3 hunks)cmd/arkd/commands.go
(1 hunks)cmd/arkd/flags.go
(1 hunks)cmd/arkd/utils.go
(1 hunks)docker-compose.regtest.yml
(2 hunks)internal/core/application/admin.go
(1 hunks)internal/core/application/indexer.go
(1 hunks)internal/core/application/service_event.go
(4 hunks)internal/core/application/utils.go
(13 hunks)internal/core/ports/tx_builder.go
(1 hunks)internal/infrastructure/db/postgres/migration/20250702115345_init.down.sql
(1 hunks)internal/infrastructure/db/postgres/migration/20250702115345_init.up.sql
(1 hunks)internal/infrastructure/db/postgres/round_repo.go
(23 hunks)internal/infrastructure/db/postgres/sqlc.yaml
(1 hunks)internal/infrastructure/db/postgres/sqlc/queries/models.go
(5 hunks)internal/infrastructure/db/postgres/sqlc/queries/query.sql.go
(1 hunks)internal/infrastructure/db/postgres/sqlc/query.sql
(1 hunks)internal/infrastructure/db/sqlite/market_hour_repo.go
(1 hunks)internal/infrastructure/db/sqlite/migration/20250702160209_init.down.sql
(1 hunks)internal/infrastructure/db/sqlite/migration/20250702160209_init.up.sql
(1 hunks)internal/infrastructure/db/sqlite/round_repo.go
(21 hunks)internal/infrastructure/db/sqlite/sqlc.yaml
(1 hunks)internal/infrastructure/db/sqlite/sqlc/queries/models.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- internal/infrastructure/db/postgres/migration/20250702115345_init.down.sql
- internal/infrastructure/db/sqlite/sqlc.yaml
- docker-compose.regtest.yml
- internal/infrastructure/db/sqlite/market_hour_repo.go
- internal/core/application/service_event.go
- internal/core/ports/tx_builder.go
- cmd/arkd/flags.go
- api-spec/protobuf/ark/v1/admin.proto
- internal/infrastructure/db/postgres/migration/20250702115345_init.up.sql
- internal/core/application/admin.go
- internal/infrastructure/db/sqlite/migration/20250702160209_init.up.sql
- internal/infrastructure/db/postgres/sqlc/query.sql
- internal/infrastructure/db/sqlite/round_repo.go
🧰 Additional context used
🧬 Code Graph Analysis (3)
internal/core/application/indexer.go (1)
internal/core/ports/repo_manager.go (1)
RepoManager
(5-12)
internal/infrastructure/db/postgres/sqlc/queries/models.go (2)
internal/config/config.go (1)
RoundInterval
(135-135)internal/infrastructure/db/sqlite/sqlc/queries/models.go (2)
Tx
(151-158)CheckpointTx
(11-17)
internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (2)
internal/infrastructure/db/postgres/sqlc/queries/models.go (10)
VtxoVw
(186-203)MarketHour
(62-69)OffchainTxVw
(81-94)Tx
(153-160)CheckpointTx
(13-19)Round
(103-115)RoundIntentsVw
(117-122)RoundTxsVw
(124-131)IntentWithReceiversVw
(51-60)IntentWithInputsVw
(28-49)internal/infrastructure/db/sqlite/sqlc/queries/models.go (10)
VtxoVw
(184-201)MarketHour
(60-67)OffchainTxVw
(79-92)Tx
(151-158)CheckpointTx
(11-17)Round
(101-113)RoundIntentsVw
(115-120)RoundTxsVw
(122-129)IntentWithReceiversVw
(49-58)IntentWithInputsVw
(26-47)
🪛 YAMLlint (1.37.1)
.github/workflows/integration.yaml
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/unit.yaml
[error] 60-60: no new line character at the end of file
(new-line-at-end-of-file)
internal/infrastructure/db/postgres/sqlc.yaml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
README.md
[uncategorized] ~129-~129: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ..._port_bitcoin ``` If none of the above option is specified, the wallet uses Neutrino ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
SECURITY.md
[misspelling] ~16-~16: Possible spelling mistake found.
Context: ...com/altafan.gpg) | 6C7639DEA147673B | | Andrew Camilleri | [https://github.com/Kukks.gpg](https:...
(EN_MULTITOKEN_SPELLING_TWO)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration tests
- GitHub Check: unit tests
🔇 Additional comments (12)
internal/infrastructure/db/sqlite/migration/20250702160209_init.down.sql (1)
1-22
: Down migration looks good
IF EXISTS
guards and the order (indexes ➜ views ➜ tables) make the rollback idempotent and dependency-safe.api-spec/openapi/swagger/ark/v1/admin.swagger.json (3)
200-230
: LGTM: Well-designed API improvementsThe changes from POST to GET for the rounds endpoint with time-based query parameters follow REST conventions and provide better querying capabilities.
352-359
: LGTM: Enhanced response with timing metadataAdding
startedAt
andendedAt
timestamp fields provides valuable timing information for round details.
512-517
: LGTM: Improved output structureSeparating
vtxoScript
andonchainAddress
into distinct fields provides clearer semantics compared to a singleaddress
field.internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
1-202
: LGTM: Well-structured database modelsThe auto-generated SQLite models correctly use nullable types and align with the domain refactoring from TxRequest to Intent and VirtualTx to OffchainTx.
cmd/arkd/utils.go (1)
317-336
: LGTM: Secure TLS configurationThe TLS configuration properly sets minimum TLS 1.2 and validates certificates using a custom CA pool.
internal/core/application/indexer.go (3)
174-177
: LGTM: Proper mutual exclusivity validationThe logic correctly ensures that spendable, spent, and recoverable filters are mutually exclusive, preventing conflicting query parameters.
247-247
: LGTM: Error message formatting fixedThe error message now correctly uses
%v
to format the slice, addressing the previous review comment about improper formatting.
396-433
: LGTM: Robust pagination implementationThe pagination function handles edge cases well, including empty results, bounds checking, and proper page calculation.
internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
130-131
: LGTM: PostgreSQL-specific JSON handlingUsing
pqtype.NullRawMessage
for JSON fields is appropriate for PostgreSQL and provides proper null handling for complex data types.internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (1)
505-598
: Indexes verified for complex joins
Appropriate indexes exist on all join keys in the PostgreSQL migrations—including
- round.id (primary key)
- intent.round_id
- tx.round_id
- receiver.intent_id
- vtxo.intent_id
No further action is required here.
internal/core/application/utils.go (1)
101-141
: LGTM! Domain model updates correctly implemented.The function properly handles the renamed fields (
ArkTx
,CommitmentTxidsList
,RootCommitmentTxId
) and uses the appropriate constants.
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: 8
♻️ Duplicate comments (3)
README.md (1)
4-4
: Fix Docker image badge referenceThe Docker image badge still references the old repository path.
-[](https://github.com/arkade-os/arkd/pkgs/container/ark) +[](https://github.com/arkade-os/arkd/pkgs/container/arkd).github/workflows/unit.yaml (1)
30-37
: Fix inconsistent job descriptionThe echo message mentions "go work sync" but the job only runs
go mod tidy
.Either update the message or add the missing command:
- name: Check for changes run: | go mod tidy if [[ -n $(git status --porcelain) ]]; then - echo "Changes detected after running go work sync and go mod tidy" + echo "Changes detected after running go mod tidy" git diff exit 1 fi.github/workflows/release.yaml (1)
23-27
: Fix YAML structure for buf_tokenThe
buf_token
is incorrectly placed outside thewith:
section.- name: Publish protos uses: bufbuild/buf-push-action@v1 with: input: api-spec/protobuf - buf_token: ${{ secrets.BUF_TOKEN }} + buf_token: ${{ secrets.BUF_TOKEN }}
🧹 Nitpick comments (6)
README.md (1)
129-129
: Fix grammatical error"option" should be "options" (plural).
-If none of the above option is specified, the wallet uses Neutrino by default with peer discovery. +If none of the above options is specified, the wallet uses Neutrino by default with peer discovery..github/workflows/artifacts.yaml (1)
41-41
: Remove trailing whitespaceoverwrite: true - +.github/workflows/unit.yaml (1)
61-61
: Add newline at end of file- name: unit testing run: make test +
.github/workflows/release.yaml (3)
29-30
: Consider adding job dependenciesThe jobs will run in parallel. If the protobuf definitions need to be published before building, consider adding dependencies.
build-and-upload: runs-on: ubuntu-latest + needs: buf-push steps: docker-build-and-push: runs-on: ubuntu-latest + needs: buf-push steps:Also applies to: 167-168
24-24
: Remove trailing whitespaceMultiple lines have trailing whitespace that should be removed.
Clean up trailing spaces on lines 24, 26, 125, and 200.
Also applies to: 26-26, 125-125, 200-200
207-207
: Add newline at end of fileAdd a newline character after the last line.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/artifacts.yaml
(1 hunks).github/workflows/release.yaml
(1 hunks).github/workflows/unit.yaml
(1 hunks)README.md
(7 hunks)cmd/arkd/commands.go
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~129-~129: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ..._port_bitcoin ``` If none of the above option is specified, the wallet uses Neutrino ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 YAMLlint (1.37.1)
.github/workflows/artifacts.yaml
[error] 41-41: trailing spaces
(trailing-spaces)
.github/workflows/release.yaml
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[error] 207-207: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/unit.yaml
[error] 61-61: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/release.yaml
27-27: unexpected key "buf_token" for "step" section. expected one of "continue-on-error", "env", "id", "if", "name", "run", "shell", "timeout-minutes", "uses", "with", "working-directory"
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration 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: 1
♻️ Duplicate comments (4)
README.md (1)
4-4
: Docker badge still points to deprecated container name
Both the shield URL and the hyperlink still useark
instead ofarkd
, so the badge is broken.-[](https://github.com/arkade-os/arkd/pkgs/container/ark) +[](https://github.com/arkade-os/arkd/pkgs/container/arkd).github/workflows/unit.yaml (2)
22-29
: Rungo work sync
before the tidy check (message currently lies)The log says it ran both
go work sync
andgo mod tidy
, but only the latter is executed.
Runninggo work sync
catches workspace drifts thatgo mod tidy
misses.- - name: Check for changes - run: | - go mod tidy + - name: Ensure workspace tidy + run: | + go work sync + go mod tidy if [[ -n $(git status --porcelain) ]]; then
41-45
: Invalid golangci-lint version (v2.1.6
) – action will 404
golangci-lint
followsv1.x.x
semantic-versioning;v2.1.6
does not exist and the job will fail at runtime.- version: v2.1.6 + # pin to the latest stable v1.x tag + version: v1.62.2.github/workflows/release.yaml (1)
24-27
:buf_token
key is mis-indented – step will fail to parse
actionlint
flags the key because it is outside thewith:
block.
Indent it two spaces so it’s a proper input tobuf-push-action
.- - name: Publish protos - uses: bufbuild/buf-push-action@v1 - with: - input: api-spec/protobuf - buf_token: ${{ secrets.BUF_TOKEN }} + - name: Publish protos + uses: bufbuild/buf-push-action@v1 + with: + input: api-spec/protobuf + buf_token: ${{ secrets.BUF_TOKEN }}
🧹 Nitpick comments (7)
README.md (4)
72-72
: Typo: “Maximume” → “Maximum”-| `ARKD_REDIS_NUM_OF_RETRIES` | Maximume number of retries for redis write operations in case of conflicts | - | +| `ARKD_REDIS_NUM_OF_RETRIES` | Maximum number of retries for Redis write operations in case of conflicts | - |
129-130
: Grammar: plural & article agreement-If none of the above option is specified, the wallet uses Neutrino by default with peer discovery. +If none of the above options are specified, the wallet falls back to Neutrino with peer discovery.
218-219
: Verb form: “setup” → “set up”-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:
278-278
: Minor wording tweak for clarity-In the `envs/` folder you can find the several dev-mode configurations for `arkd` and `arkd-wallet`. +The `envs/` folder contains several dev-mode configurations for `arkd` and `arkd-wallet`..github/workflows/artifacts.yaml (1)
40-42
: Remove stray trailing spaces to satisfy YAML lintersLine 41 contains only whitespace and triggers
trailing-spaces
violations (yamllint
).
Safe to delete – keeps the workflow green and avoids needless churn in future diffs.- overwrite: true - + overwrite: true.github/workflows/unit.yaml (1)
52-53
: Add missing newline at end-of-fileSeveral linters flag this; cheap to fix and keeps diffs cleaner.
- run: make test + run: make test +.github/workflows/release.yaml (1)
190-190
: Trailing spaces & final newlineA few lines (24, 26, 125, 200) still have trailing spaces; the file also lacks a terminating newline. Cleaning these avoids noisy diffs and linter failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/artifacts.yaml
(1 hunks).github/workflows/release.yaml
(1 hunks).github/workflows/unit.yaml
(1 hunks)README.md
(7 hunks)cmd/arkd/commands.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/arkd/commands.go
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/artifacts.yaml
[error] 41-41: trailing spaces
(trailing-spaces)
.github/workflows/release.yaml
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[error] 207-207: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/unit.yaml
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.7)
.github/workflows/release.yaml
27-27: unexpected key "buf_token" for "step" section. expected one of "continue-on-error", "env", "id", "if", "name", "run", "shell", "timeout-minutes", "uses", "with", "working-directory"
(syntax-check)
🪛 LanguageTool
README.md
[uncategorized] ~129-~129: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ..._port_bitcoin ``` If none of the above option is specified, the wallet uses Neutrino ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration tests
🔇 Additional comments (1)
.github/workflows/release.yaml (1)
183-206
: Nice upgrade todocker/build-push-action@v6
Action version bump + multi-arch build args look good and align with other workflows.
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
♻️ Duplicate comments (2)
README.md (2)
4-4
: Docker badge still points to the oldark
imageThe badge and link should reference
arkd
, not the deprecatedark
image.-[](https://github.com/arkade-os/arkd/pkgs/container/ark) +[](https://github.com/arkade-os/arkd/pkgs/container/arkd)
68-68
: Event-DB URL description contradicts the allowed typesThe table says the type can only be
redis
orinmemory
, yet the URL description talks aboutpostgres
. Clarify which backend is actually supported and align the wording.-| `ARKD_EVENT_DB_URL` | Event database url if `ARKD_EVENT_DB_TYPE` is set to `postgres` | - | +| `ARKD_EVENT_DB_URL` | Event database URL if `ARKD_EVENT_DB_TYPE` is set to `redis` | - |
🧹 Nitpick comments (3)
README.md (3)
71-72
: Spelling/wording fixMinor typo and style improvements.
-| `ARKD_REDIS_NUM_OF_RETRIES` | Maximume number of retries for redis write operations in case of conflicts | - | +| `ARKD_REDIS_NUM_OF_RETRIES` | Maximum number of retries for Redis write operations in case of conflicts | - |
77-77
: Remove stray space and capitalise sentence-| `ARKD_WALLET_ADDR` | the arkd wallet address to connect to in the form `host:port` | - | +| `ARKD_WALLET_ADDR` | The arkd wallet address to connect to in the form `host:port` | - |
129-129
: Grammar fix-If none of the above option is specified, the wallet uses Neutrino by default with peer discovery. +If none of the above options are specified, the wallet uses Neutrino by default with peer discovery.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~129-~129: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ..._port_bitcoin ``` If none of the above option is specified, the wallet uses Neutrino ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration 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
♻️ Duplicate comments (2)
README.md (2)
4-4
: Docker badge still points to the legacyark
containerBoth the badge image URL and the link target should reference the new
arkd
container path.-[](https://github.com/arkade-os/arkd/pkgs/container/ark) +[](https://github.com/arkade-os/arkd/pkgs/container/arkd)
68-68
: Inconsistent database type inARKD_EVENT_DB_URL
descriptionThe preceding row limits
ARKD_EVENT_DB_TYPE
toredis
orinmemory
, yet the URL description mentionspostgres
.-| `ARKD_EVENT_DB_URL` | Event database url if `ARKD_EVENT_DB_TYPE` is set to `postgres` | - | +| `ARKD_EVENT_DB_URL` | Event database URL if `ARKD_EVENT_DB_TYPE` is set to `redis` | - |
🧹 Nitpick comments (4)
README.md (4)
8-8
: Go Reference badge is not clickableThe badge is rendered as a plain image. Wrap it in a link so that readers can reach the pkg.go.dev page directly.
- +[](https://pkg.go.dev/github.com/arkade-os/arkd)
182-187
: Nested bullets need proper indentationThe sub-packages under
pkg/
appear at the same indentation level as the parent, which renders as a flat list instead of a nested one. Indent the child items by two spaces.- - [`ark-lib`](./pkg/ark-lib): collection of data structures and functions reusable by arkd and sdk. - - [`arkd-wallet`](./pkg/arkd-wallet): bitcoin wallet service used as liquidity provider and signer. - - [`ark-cli`](./pkg/ark-cli): ark offchain and onchain wallet as command line interface. + - [`ark-lib`](./pkg/ark-lib): collection of data structures and functions reusable by arkd and sdk. + - [`arkd-wallet`](./pkg/arkd-wallet): bitcoin wallet service used as liquidity provider and signer. + - [`ark-cli`](./pkg/ark-cli): ark off-chain and on-chain wallet CLI.
189-189
: Line-number anchor is brittleThe link
README.md#L19-L22
will break as soon as the README changes again. Prefer linking to a stable heading anchor instead.
218-218
: Use “set up” (verb) instead of “setup” (noun)-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration tests
🔇 Additional comments (1)
README.md (1)
216-216
: Go 1.23 is not released yetSpecify “Go 1.22+” (latest stable at the time of writing) or simply “latest stable Go” to avoid confusion.
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: 2
🧹 Nitpick comments (2)
README.md (2)
218-219
: Verb form: “set up”, not “setup”Minor grammar tweak for clarity.
-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:
8-8
: Make the Go Reference badge clickableLinking the badge improves usability.
- +[](https://pkg.go.dev/github.com/arkade-os/arkd)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration tests
- GitHub Check: unit 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: 2
♻️ Duplicate comments (1)
README.md (1)
67-68
: Wrong condition forARKD_EVENT_DB_URL
The description still references
postgres
, butARKD_EVENT_DB_TYPE
only listsredis
|inmemory
. This was flagged in previous reviews and is still unresolved.-| `ARKD_EVENT_DB_URL` | Event database url if `ARKD_EVENT_DB_TYPE` is set to `postgres` | - | +| `ARKD_EVENT_DB_URL` | Event database URL if `ARKD_EVENT_DB_TYPE` is set to `redis` | - |
🧹 Nitpick comments (2)
README.md (2)
189-191
: Line-number URLs are fragileLinking to
README.md#L19-L22
will break as soon as the file shifts. Prefer section-anchor links.- - [`application`](./internal/core/application/): contains the implementation of the service responsible for the [core operations](https://github.com/arkade-os/arkd/tree/master/README.md#L19-L22). + - [`application`](./internal/core/application/): contains the implementation of the service responsible for the [core operations](#what-is-arkd).
218-219
: Minor grammar: “set up” (verb) not “setup”-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration 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
♻️ Duplicate comments (2)
README.md (2)
6-6
: Drop or repoint the stale unit-test workflow badge
unit.yaml
was removed earlier, yet the badge still links to it.
Keep the README accurate by either-[](https://github.com/arkade-os/arkd/actions/workflows/unit.yaml) +<!-- Unit-test workflow removed in v7; re-add badge once reinstated -->or by targeting an existing workflow.
Broken badges look unmaintained.
67-68
: Wrong condition forARKD_EVENT_DB_URL
– references the wrong DB typeThe URL is used only when the event DB is Redis, not Postgres.
-| `ARKD_EVENT_DB_URL` | Event database url if `ARKD_EVENT_DB_TYPE` is set to `postgres` | - | +| `ARKD_EVENT_DB_URL` | Redis connection URL if `ARKD_EVENT_DB_TYPE` is set to `redis` | - |Persisting the old wording keeps the docs misleading and contradicts the enum two lines above.
🧹 Nitpick comments (2)
README.md (2)
73-73
: 7-day expiry constant is off by 128 seconds
604672
s ≈ 6 days 23 h 51 m 12 s.
If you really mean a full week, use 604 800 s.-| `ARKD_VTXO_TREE_EXPIRY` | VTXO tree expiry in seconds | `604672` (7 days) | +| `ARKD_VTXO_TREE_EXPIRY` | VTXO tree expiry in seconds | `604800` (7 days) |
218-218
: Verb/noun confusion: “setup” → “set up”-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:Small polish; improves readability and passes spell-checkers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit tests
- GitHub Check: integration tests
🔇 Additional comments (1)
README.md (1)
216-216
: Verify the stated Go requirement – 1.23 is not releasedGo 1.22 is the latest at the time of writing. Re-check the minimum version and update before tagging v7.
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
♻️ Duplicate comments (2)
README.md (2)
6-6
: Unit-test badge still points to removedunit.yaml
workflow
unit.yaml
no longer exists, so this badge will 404 once the branch is merged. Either restore the workflow (with the exact name) or drop / update the badge.-[](https://github.com/arkade-os/arkd/actions/workflows/unit.yaml) +<!-- Unit-test workflow removed in v7; re-add badge once reinstated -->
68-68
: Wrong condition forARKD_EVENT_DB_URL
The URL is used when the event DB type is
redis
, notpostgres
.-| `ARKD_EVENT_DB_URL` | Event database url if `ARKD_EVENT_DB_TYPE` is set to `postgres` | - | +| `ARKD_EVENT_DB_URL` | Event database URL if `ARKD_EVENT_DB_TYPE` is set to `redis` | - |
🧹 Nitpick comments (2)
README.md (2)
218-218
: Grammar nit – use “set up” (verb) instead of “setup” (noun)-3. Start Nigiri to setup a regtest Bitcoin environment: +3. Start Nigiri to set up a regtest Bitcoin environment:
269-269
: Line-number anchor in integration-test link is brittleThe link relies on a hard-coded
#L218
anchor which will break every time the README changes. Point to a section heading instead, e.g.:-3. Run integration tests ([start nigiri](https://github.com/arkade-os/arkd/tree/master/README.md#L218) if needed first): +3. Run integration tests ([start Nigiri](#3-start-nigiri-to-set-up-a-regtest-bitcoin-environment) if needed first):(or any stable local heading).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(7 hunks)cmd/arkd/commands.go
(1 hunks)cmd/arkd/flags.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/arkd/flags.go
- cmd/arkd/commands.go
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~182-~182: Loose punctuation mark.
Context: ...y Structure - api-spec
: Ark Protocol Buffer API specification. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~183-~183: Loose punctuation mark.
Context: ...fer API specification. - pkg
: collection of reusable packages and ser...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...services. - ark-lib
: collection of data structures and funct...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: .... - arkd-wallet
: bitcoin wallet service used as liquidit...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...d signer. - ark-cli
: ark offchain and onchain wallet as comm...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...ne interface. - internal
: arkd implementation. - [core
](./int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~188-~188: Loose punctuation mark.
Context: ...mentation. - core
: contains the core business logic of ark...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~189-~189: Loose punctuation mark.
Context: ...lication`](./internal/core/application/): contains the implementation of the serv...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~190-~190: Loose punctuation mark.
Context: ... - domain
: models and events managed by the applic...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~191-~191: Loose punctuation mark.
Context: ... - ports
: collection of interfaces of the service...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~192-~192: Loose punctuation mark.
Context: ...astructure`](./internal/infrastructure/): contains implementations of the interfa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~193-~193: Loose punctuation mark.
Context: ... - interface
: contains the implementations of the int...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~195-~195: Loose punctuation mark.
Context: ...ST endpoints. - test/e2e
: contains the integration tests. ## Dev...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~203-~203: Loose punctuation mark.
Context: ... root of the repository: - make build
: Builds the arkd
binary for your platf...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~204-~204: Loose punctuation mark.
Context: ...ry for your platform. - make build-all
: Builds the arkd
binary for all platfo...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~218-~218: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...cal Bitcoin networks 3. Start Nigiri to setup a regtest Bitcoin environment: ```s...
(NOUN_VERB_CONFUSION)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...ake docker-stop ``` In the envs/
folder you can find the several dev-mode confi...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration tests
- GitHub Check: unit tests
This is the final refactor before releasing v7.
It contains mainly renamings in
server/
to ensure we stick as much as possible with the terms used in the docs.This also adds some side changes to tidy up the admin proto and adds commands to the arkd cli in order to cover all apis exposed by the admin interface.
Please @louisinger review (😂)
Summary by CodeRabbit
Here are the updated release notes based on the provided changes:
New Features
Improvements
Bug Fixes
Chores
Style
Documentation