hotfix/v2.2/inconsistent ledger creation #700
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces extensive updates across documentation, internal APIs, SDK clients, and tests to support Ledger API version 2. Key changes include updating documentation to reflect versioning (e.g., from “ledger” to “ledger.v2”), converting ID fields from int to pointer types with added Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as NewRouter (V2)
participant API as API Endpoint
C->>R: Call NewRouter(systemController, auth, "develop", debug)
R->>R: Initialize routes & options (bulk handlers, version setting)
R->>API: Map GET /_info to GetInfo handler
API-->>R: Return Ledger Info Response
R-->>C: Pass response back to client
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked 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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
pkg/client/v2.go (1)
30-4912: 🛠️ Refactor suggestionGeneral observation on repeated boilerplate.
Across all methods, you replicate blocks for:
• Timeout & retry config
• Security & hooking
• Response reading & content-type checksRefactoring into shared helper routines could significantly reduce code size and the risk of inconsistencies or mistakes.
internal/storage/bucket/migrations/11-make-stateless/up.sql (1)
433-471:⚠️ Potential issueCritical: Log Hash Generation
The
set_log_hash()function constructs a JSON representation (including selective fields) and computes a SHA-256 digest. The conditional handling—using the previous hash if present—is clever. Please verify that the concatenation (with newlines as delimiters) and the encoding steps match the overall system’s verification requirements.
🧹 Nitpick comments (29)
internal/storage/driver/driver.go (2)
46-51: Consider logging additional context before returning.
IfsystemStore.CreateLedgerreturns an unexpected error, you're wrapping but not logging any details of the failure path. Adding logs (or relevant telemetry) can help debug environmental or concurrency-related failures.
68-70: Watch out for partial completion.
If adding the ledger to the bucket fails here, the ledger remains created in the system store, leaving a possible partial/inconsistent state. Consider rolling back the ledger creation or providing a clear cleanup path if this step fails.internal/storage/ledger/legacy/adapters.go (5)
76-78: Panic in GetOne.
You rely on “never used” to justify a panic. Safeguard by returning an error or explaining in a comment why this is guaranteed never to be invoked.
80-82: Same pattern for Count.
Use a consistent approach to “never used” methods. Consider returning a descriptive error instead of panicking.
183-185: Count panics.
Similar note—replace with error return to avoid unexpected application crashes.
208-210: Panic in GetOne.
Match the approach used elsewhere—avoid panics where possible.
212-214: Panic in Count.
Same feedback as above—prefer an error to a panic.pkg/client/v2.go (4)
35-36: Avoid duplicating the same OAuth2 scope.The array includes
"ledger:read"twice, which seems redundant. Consider removing the duplicate scope for clarity:- OAuth2Scopes: []string{"ledger:read", "ledger:read"}, + OAuth2Scopes: []string{"ledger:read"},
68-128: Consolidate repeated request handling and retry logic.Within this block, logic for building and executing requests (including retry configuration, hook calls, and response handling) is repeated in every method. Extracting it into a shared helper function would reduce duplication, make the code more DRY, and improve maintainability.
221-226: Maintain consistent naming for Operation IDs.Here, the
OperationIDis"getMetrics"while the previous method used"v2GetInfo"with a version prefix. Unifying these naming conventions (e.g.,"v2GetMetrics") would enhance clarity and keep version references consistent.
355-395: Consider returning a typed struct for metrics.Currently, metrics are unmarshaled into a
map[string]any. If you have a known, stable schema, consider defining a dedicated struct for better type safety and clarity. Fallback tomap[string]anycan be a last resort if the data is highly dynamic.internal/api/v2/controllers_stats_test.go (1)
20-20: Consider adding error test cases.While the happy path is tested, consider adding test cases for error scenarios such as:
- Invalid ledger name
- Database errors
- Stats calculation errors
internal/api/v2/controllers_ledgers_read_test.go (1)
25-25: Enhance test coverage with version-specific assertions.While the basic functionality is tested, consider adding assertions for:
- API version headers in the response
- Error cases (e.g., non-existent ledger, invalid ledger name)
- Version compatibility checks
Example test enhancement:
router.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) +require.Equal(t, "v2", rec.Header().Get("X-API-Version")) ledgerFromAPI, _ := api.DecodeSingleResponse[ledger.Ledger](t, rec.Body) require.Equal(t, l, ledgerFromAPI)internal/api/v2/controllers_transactions_read_test.go (1)
31-31: Consider using dynamic transaction ID instead of hardcoded value.Using a hardcoded value
0makes the test less flexible and might not catch edge cases. Consider usingtx.IDto maintain the test's robustness.- Builder: query.Match("id", 0), + Builder: query.Match("id", tx.ID),internal/api/v1/controllers_config.go (1)
35-35: Add documentation for the exported function.Since
GetInfois now exported, it should have proper documentation explaining its purpose, parameters, and return values.+// GetInfo returns a handler function that provides server configuration information. +// It uses the provided systemController to list available ledgers and returns +// a ConfigInfo structure containing server details, version, and ledger configuration. +// +// Parameters: +// - systemController: Controller interface for accessing system functionality +// - version: String indicating the server version +// +// Returns: An http.HandlerFunc that writes the configuration info to the response func GetInfo(systemController system.Controller, version string) func(w http.ResponseWriter, r *http.Request) {internal/machine/vm/run.go (1)
38-43: Enhance type handling for amount values.While the current implementation handles string and float64 types, consider making it more robust by:
- Adding support for other numeric types (int, int64, etc.).
- Adding error handling for invalid type conversions.
switch amount := v["amount"].(type) { case string: s.Script.Vars[k] = fmt.Sprintf("%s %s", v["asset"], amount) case float64: s.Script.Vars[k] = fmt.Sprintf("%s %d", v["asset"], int(amount)) +case int: + s.Script.Vars[k] = fmt.Sprintf("%s %d", v["asset"], amount) +case int64: + s.Script.Vars[k] = fmt.Sprintf("%s %d", v["asset"], amount) +default: + return Script{}, fmt.Errorf("unsupported amount type: %T", amount) }pkg/testserver/helpers.go (1)
5-5: LGTM! Consider adding null check.The conversion to pointer-based ID is implemented correctly. However, consider adding a null check for tx.ID before conversion.
- ID: pointer.For(int(tx.ID.Int64())), + ID: func() *int { + if tx.ID == nil { + return nil + } + return pointer.For(int(tx.ID.Int64())) + }(),Also applies to: 39-39
internal/storage/ledger/legacy/transactions_test.go (1)
50-52: Add test cases for nil transaction IDs.The test cases verify transaction functionality with valid IDs but don't cover scenarios where transaction IDs are nil. Consider adding test cases to verify proper handling of nil IDs.
Example test case to add:
func TestGetTransactionWithVolumesNilID(t *testing.T) { t.Parallel() store := newLedgerStore(t) ctx := logging.TestingContext() tx := ledger.NewTransaction(). WithPostings( ledger.NewPosting("world", "central_bank", "USD", big.NewInt(100)), ) tx.ID = nil err := store.newStore.CommitTransaction(ctx, &tx) require.NoError(t, err) require.NotNil(t, tx.ID, "Expected transaction ID to be assigned") }Also applies to: 72-74, 146-146, 154-154, 162-162
test/e2e/api_ledgers_import_test.go (1)
56-60: Consider using a separate test data file.The test data contains multiple JSON log entries directly in the code. Consider moving this data to a separate test fixture file for better maintainability.
- logs := `{"type":"NEW_TRANSACTION"...}` + logs, err := os.ReadFile("testdata/v2.1_import_logs.json") + require.NoError(t, err)pkg/client/README.md (2)
112-113: Standardize Unordered List FormattingConsider adopting hyphens (
-) for unordered list items instead of asterisks (*) to meet markdownlint recommendations (MD004) for consistent style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
112-112: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
113-113: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
1-461: Consistent Markdown FormattingThere are multiple instances of hard tab characters within code blocks. Replacing hard tabs with spaces will help conform to markdownlint guidelines (MD010) and enhance overall readability.
🧰 Tools
🪛 LanguageTool
[style] ~369-~369: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...he requirements for the HTTP client are very simple. It must match this interface: ```go t...(EN_WEAK_ADJECTIVE)
[uncategorized] ~377-~377: When a number forms part of an adjectival compound, use a hyphen.
Context: ...ple example, which adds a client with a 30 second timeout. ```go import ( "net/http" "...(MISSING_HYPHEN)
[style] ~458-~458: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ... look forward to hearing your feedback. Feel free to open a PR or an issue with a proof of c...(FEEL_FREE_TO_STYLE_ME)
[uncategorized] ~458-~458: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...a PR or an issue with a proof of concept and we'll do our best to include it in a fu...(COMMA_COMPOUND_SENTENCE)
🪛 markdownlint-cli2 (0.17.2)
6-6: Images should have alternate text (alt text)
null(MD045, no-alt-text)
8-8: Images should have alternate text (alt text)
null(MD045, no-alt-text)
30-30: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
31-31: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
32-32: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
33-33: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
34-34: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
35-35: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
36-36: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
37-37: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
57-57: Hard tabs
Column: 1(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1(MD010, no-hard-tabs)
69-69: Hard tabs
Column: 1(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1(MD010, no-hard-tabs)
74-74: Hard tabs
Column: 1(MD010, no-hard-tabs)
75-75: Hard tabs
Column: 1(MD010, no-hard-tabs)
76-76: Hard tabs
Column: 1(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1(MD010, no-hard-tabs)
89-89: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
90-90: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
91-91: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
92-92: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
93-93: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
94-94: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
95-95: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
96-96: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
97-97: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
98-98: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
99-99: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
100-100: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
101-101: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
102-102: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
103-103: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
104-104: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
105-105: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
106-106: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
107-107: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
108-108: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
112-112: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
113-113: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
114-114: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
115-115: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
116-116: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
117-117: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
118-118: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
119-119: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
120-120: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
121-121: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
122-122: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
123-123: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
124-124: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
125-125: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
126-126: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
127-127: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
128-128: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
129-129: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
130-130: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
131-131: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
132-132: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
133-133: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
134-134: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
135-135: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
136-136: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
137-137: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
138-138: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
151-151: Hard tabs
Column: 1(MD010, no-hard-tabs)
152-152: Hard tabs
Column: 1(MD010, no-hard-tabs)
153-153: Hard tabs
Column: 1(MD010, no-hard-tabs)
154-154: Hard tabs
Column: 1(MD010, no-hard-tabs)
155-155: Hard tabs
Column: 1(MD010, no-hard-tabs)
156-156: Hard tabs
Column: 1(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 1(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 1(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 1(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 1(MD010, no-hard-tabs)
177-177: Hard tabs
Column: 1(MD010, no-hard-tabs)
178-178: Hard tabs
Column: 1(MD010, no-hard-tabs)
179-179: Hard tabs
Column: 1(MD010, no-hard-tabs)
180-180: Hard tabs
Column: 1(MD010, no-hard-tabs)
181-181: Hard tabs
Column: 1(MD010, no-hard-tabs)
182-182: Hard tabs
Column: 1(MD010, no-hard-tabs)
183-183: Hard tabs
Column: 1(MD010, no-hard-tabs)
184-184: Hard tabs
Column: 1(MD010, no-hard-tabs)
194-194: Hard tabs
Column: 1(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 1(MD010, no-hard-tabs)
196-196: Hard tabs
Column: 1(MD010, no-hard-tabs)
197-197: Hard tabs
Column: 1(MD010, no-hard-tabs)
198-198: Hard tabs
Column: 1(MD010, no-hard-tabs)
202-202: Hard tabs
Column: 1(MD010, no-hard-tabs)
203-203: Hard tabs
Column: 1(MD010, no-hard-tabs)
204-204: Hard tabs
Column: 1(MD010, no-hard-tabs)
205-205: Hard tabs
Column: 1(MD010, no-hard-tabs)
206-206: Hard tabs
Column: 1(MD010, no-hard-tabs)
207-207: Hard tabs
Column: 1(MD010, no-hard-tabs)
208-208: Hard tabs
Column: 1(MD010, no-hard-tabs)
209-209: Hard tabs
Column: 1(MD010, no-hard-tabs)
210-210: Hard tabs
Column: 1(MD010, no-hard-tabs)
211-211: Hard tabs
Column: 1(MD010, no-hard-tabs)
212-212: Hard tabs
Column: 1(MD010, no-hard-tabs)
213-213: Hard tabs
Column: 1(MD010, no-hard-tabs)
214-214: Hard tabs
Column: 1(MD010, no-hard-tabs)
215-215: Hard tabs
Column: 1(MD010, no-hard-tabs)
216-216: Hard tabs
Column: 1(MD010, no-hard-tabs)
217-217: Hard tabs
Column: 1(MD010, no-hard-tabs)
218-218: Hard tabs
Column: 1(MD010, no-hard-tabs)
220-220: Hard tabs
Column: 1(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 1(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1(MD010, no-hard-tabs)
223-223: Hard tabs
Column: 1(MD010, no-hard-tabs)
224-224: Hard tabs
Column: 1(MD010, no-hard-tabs)
225-225: Hard tabs
Column: 1(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1(MD010, no-hard-tabs)
249-249: Hard tabs
Column: 1(MD010, no-hard-tabs)
250-250: Hard tabs
Column: 1(MD010, no-hard-tabs)
251-251: Hard tabs
Column: 1(MD010, no-hard-tabs)
252-252: Hard tabs
Column: 1(MD010, no-hard-tabs)
253-253: Hard tabs
Column: 1(MD010, no-hard-tabs)
254-254: Hard tabs
Column: 1(MD010, no-hard-tabs)
258-258: Hard tabs
Column: 1(MD010, no-hard-tabs)
259-259: Hard tabs
Column: 1(MD010, no-hard-tabs)
260-260: Hard tabs
Column: 1(MD010, no-hard-tabs)
261-261: Hard tabs
Column: 1(MD010, no-hard-tabs)
262-262: Hard tabs
Column: 1(MD010, no-hard-tabs)
263-263: Hard tabs
Column: 1(MD010, no-hard-tabs)
265-265: Hard tabs
Column: 1(MD010, no-hard-tabs)
266-266: Hard tabs
Column: 1(MD010, no-hard-tabs)
267-267: Hard tabs
Column: 1(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 1(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 1(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 1(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 1(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 1(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 1(MD010, no-hard-tabs)
276-276: Hard tabs
Column: 1(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 1(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 1(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 1(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 1(MD010, no-hard-tabs)
303-303: Hard tabs
Column: 1(MD010, no-hard-tabs)
304-304: Hard tabs
Column: 1(MD010, no-hard-tabs)
305-305: Hard tabs
Column: 1(MD010, no-hard-tabs)
306-306: Hard tabs
Column: 1(MD010, no-hard-tabs)
310-310: Hard tabs
Column: 1(MD010, no-hard-tabs)
311-311: Hard tabs
Column: 1(MD010, no-hard-tabs)
312-312: Hard tabs
Column: 1(MD010, no-hard-tabs)
313-313: Hard tabs
Column: 1(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1(MD010, no-hard-tabs)
315-315: Hard tabs
Column: 1(MD010, no-hard-tabs)
316-316: Hard tabs
Column: 1(MD010, no-hard-tabs)
318-318: Hard tabs
Column: 1(MD010, no-hard-tabs)
319-319: Hard tabs
Column: 1(MD010, no-hard-tabs)
320-320: Hard tabs
Column: 1(MD010, no-hard-tabs)
321-321: Hard tabs
Column: 1(MD010, no-hard-tabs)
322-322: Hard tabs
Column: 1(MD010, no-hard-tabs)
323-323: Hard tabs
Column: 1(MD010, no-hard-tabs)
324-324: Hard tabs
Column: 1(MD010, no-hard-tabs)
325-325: Hard tabs
Column: 1(MD010, no-hard-tabs)
338-338: Hard tabs
Column: 1(MD010, no-hard-tabs)
339-339: Hard tabs
Column: 1(MD010, no-hard-tabs)
340-340: Hard tabs
Column: 1(MD010, no-hard-tabs)
341-341: Hard tabs
Column: 1(MD010, no-hard-tabs)
345-345: Hard tabs
Column: 1(MD010, no-hard-tabs)
346-346: Hard tabs
Column: 1(MD010, no-hard-tabs)
347-347: Hard tabs
Column: 1(MD010, no-hard-tabs)
348-348: Hard tabs
Column: 1(MD010, no-hard-tabs)
349-349: Hard tabs
Column: 1(MD010, no-hard-tabs)
350-350: Hard tabs
Column: 1(MD010, no-hard-tabs)
351-351: Hard tabs
Column: 1(MD010, no-hard-tabs)
353-353: Hard tabs
Column: 1(MD010, no-hard-tabs)
354-354: Hard tabs
Column: 1(MD010, no-hard-tabs)
355-355: Hard tabs
Column: 1(MD010, no-hard-tabs)
356-356: Hard tabs
Column: 1(MD010, no-hard-tabs)
357-357: Hard tabs
Column: 1(MD010, no-hard-tabs)
358-358: Hard tabs
Column: 1(MD010, no-hard-tabs)
359-359: Hard tabs
Column: 1(MD010, no-hard-tabs)
360-360: Hard tabs
Column: 1(MD010, no-hard-tabs)
373-373: Hard tabs
Column: 1(MD010, no-hard-tabs)
381-381: Hard tabs
Column: 1(MD010, no-hard-tabs)
382-382: Hard tabs
Column: 1(MD010, no-hard-tabs)
383-383: Hard tabs
Column: 1(MD010, no-hard-tabs)
387-387: Hard tabs
Column: 1(MD010, no-hard-tabs)
388-388: Hard tabs
Column: 1(MD010, no-hard-tabs)
418-418: Hard tabs
Column: 1(MD010, no-hard-tabs)
419-419: Hard tabs
Column: 1(MD010, no-hard-tabs)
420-420: Hard tabs
Column: 1(MD010, no-hard-tabs)
421-421: Hard tabs
Column: 1(MD010, no-hard-tabs)
425-425: Hard tabs
Column: 1(MD010, no-hard-tabs)
426-426: Hard tabs
Column: 1(MD010, no-hard-tabs)
427-427: Hard tabs
Column: 1(MD010, no-hard-tabs)
428-428: Hard tabs
Column: 1(MD010, no-hard-tabs)
429-429: Hard tabs
Column: 1(MD010, no-hard-tabs)
430-430: Hard tabs
Column: 1(MD010, no-hard-tabs)
432-432: Hard tabs
Column: 1(MD010, no-hard-tabs)
433-433: Hard tabs
Column: 1(MD010, no-hard-tabs)
434-434: Hard tabs
Column: 1(MD010, no-hard-tabs)
435-435: Hard tabs
Column: 1(MD010, no-hard-tabs)
436-436: Hard tabs
Column: 1(MD010, no-hard-tabs)
437-437: Hard tabs
Column: 1(MD010, no-hard-tabs)
438-438: Hard tabs
Column: 1(MD010, no-hard-tabs)
439-439: Hard tabs
Column: 1(MD010, no-hard-tabs)
internal/storage/bucket/migrations/11-make-stateless/up.sql (1)
196-207: Table Structure Modifications for Transactions and LogsAdding the
post_commit_volumesandinserted_atcolumns to thetransactionstable (and adjusting thelogstable accordingly) is essential. Note the commented out TODO regarding converting theidcolumn type; further consideration may be needed if full migration is desired.internal/README.md (4)
478-480: Log struct: Update ID field to pointer type
The addition of theIDfield as a pointer (*int) is consistent with the new design that allows a nil value. Please ensure that all consumers of theLogstruct are updated accordingly and that the documentation reflects thatIDmay be nil.
519-525: Add method: Log.WithID
The newly introducedWithIDmethod for theLogtype cleanly encapsulates the creation of a modified log with a specified ID. It might be beneficial to add a brief comment (or documentation comment) above the method declaration to explain that it returns a new copy of the log with the provided ID.
931-935: Add method: Transaction.WithID
The addition ofWithIDto theTransactiontype mirrors the change made forLogand supports consistent ID handling across exported entities. As withLog.WithID, consider adding a short documentation comment describing the method’s purpose.
44-47: Markdown Formatting: Replace hard tabs and add blank lines around tables
Static analysis detected several instances of hard tabs (e.g. lines 44–47 and 95–98) and tables that are not surrounded by blank lines (e.g. line 131). Converting hard tabs to spaces and ensuring that tables have a blank line before and after would improve the markdown readability and adherence to MD010 and MD058 guidelines.Also applies to: 95-98, 131-131
pkg/client/docs/sdks/v2/README.md (2)
34-39: GetInfo section: Clarity and example usage
The newly added GetInfo section succinctly describes that this operation shows server information and includes a concise example usage snippet. Ensure that the description (e.g. “Show server information”) clearly matches the behavior of the endpoint and that potential error cases are documented elsewhere in the SDK reference.
85-92: GetMetrics section: Documentation and example usage
The introduction of the GetMetrics section is well done: it explains that the endpoint reads in-memory metrics and provides a complete example usage snippet. Verify that the response details (e.g. response type and error handling) are consistent with the SDK’s overall style. Consider adding one more sentence (if needed) to clarify the metrics’ context.docs/api/README.md (1)
22-22: Header Update for Ledger API VersioningThe new header
<h1 id="ledger-api-ledger-v2">ledger.v2</h1>clearly marks the transition to version 2 of the ledger API. This change helps in differentiating the new version’s documentation from earlier iterations and aligns with the PR objective of addressing inconsistencies in ledger creation.A couple of points for consideration:
- Styling Consistency: Ensure the header’s text case and style are consistent with the rest of the document (for example, comparing with
<h1 id="ledger-api">Ledger API v2</h1>on line 3).- Semantic Clarity: Verify that the new header’s identifier (
id="ledger-api-ledger-v2") is used uniformly throughout the documentation for internal linking and navigation.
🛑 Comments failed to post (4)
internal/storage/driver/driver.go (1)
72-75: 🛠️ Refactor suggestion
Ensure consistent rollback strategy.
Migrating the bucket after the ledger is created in the system store might lead to inconsistencies if migration fails. A well-defined rollback or retry mechanism could mitigate this risk.internal/api/bulking/bulker.go (1)
144-144:
⚠️ Potential issueAdd nil checks before dereferencing
log.ID.The code assumes that
log.IDis never nil. However, sinceIDis now a pointer type, it's important to add nil checks to prevent potential panics.Apply this diff to add nil checks:
- return createTransactionResult.Transaction, *log.ID, nil + if log.ID == nil { + return nil, 0, fmt.Errorf("log ID is nil") + } + return createTransactionResult.Transaction, *log.ID, nil - return nil, *log.ID, nil + if log.ID == nil { + return nil, 0, fmt.Errorf("log ID is nil") + } + return nil, *log.ID, nil - return revertTransactionResult.RevertedTransaction, *log.ID, nil + if log.ID == nil { + return nil, 0, fmt.Errorf("log ID is nil") + } + return revertTransactionResult.RevertedTransaction, *log.ID, nil - return nil, *log.ID, nil + if log.ID == nil { + return nil, 0, fmt.Errorf("log ID is nil") + } + return nil, *log.ID, nilAlso applies to: 186-186, 203-203, 247-247
internal/storage/ledger/transactions.go (2)
77-78:
⚠️ Potential issueAdd nil check before dereferencing transaction ID in move creation.
The code assumes that
tx.IDis not nil when creating moves. However, sinceIDis now a pointer type, it's important to add nil checks to prevent potential panics.Apply this diff to add nil checks:
+ if tx.ID == nil { + return fmt.Errorf("transaction ID is nil") + } moves = append(moves, &ledger.Move{ Account: posting.Destination, Amount: (*bunpaginate.BigInt)(posting.Amount), Asset: posting.Asset, InsertionDate: tx.InsertedAt, EffectiveDate: tx.Timestamp, PostCommitVolumes: pointer.For(postCommitVolumes[posting.Destination][posting.Asset].Copy()), TransactionID: *tx.ID, }) postCommitVolumes.AddInput(posting.Destination, posting.Asset, new(big.Int).Neg(posting.Amount)) + if tx.ID == nil { + return fmt.Errorf("transaction ID is nil") + } moves = append(moves, &ledger.Move{ IsSource: true, Account: posting.Source, Amount: (*bunpaginate.BigInt)(posting.Amount), Asset: posting.Asset, InsertionDate: tx.InsertedAt, EffectiveDate: tx.Timestamp, PostCommitVolumes: pointer.For(postCommitVolumes[posting.Source][posting.Asset].Copy()), TransactionID: *tx.ID, })Also applies to: 89-90
142-142:
⚠️ Potential issueAdd nil check before dereferencing transaction ID in tracing.
The code assumes that
tx.IDis not nil when setting trace attributes. Add a nil check to prevent potential panics.Apply this diff to add nil checks:
- attribute.Int("id", *tx.ID), + attribute.Int("id", pointer.GetOrDefault(tx.ID, 0)),📝 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.attribute.Int("id", pointer.GetOrDefault(tx.ID, 0)),
No description provided.