fix: ledger creations atomic (v2.2) #710
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 WalkthroughThe changes remove the dependency on the old Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant D as Driver
participant DB as Database
participant BF as BucketFactory
C->>D: New(db, ledgerFactory, bucketFactory, ...)
D->>DB: Begin transaction
D->>BF: Create Bucket (without passing DB)
D->>DB: Run migrations & create ledger within transaction
DB-->>D: Transaction committed
D-->>C: Returns driver instance
sequenceDiagram
participant Cl as Ledger Client
participant RT as Router (API V2)
participant Srv as Server
Cl->>RT: GET /_info with version "develop"
RT->>Srv: Call v1.GetInfo(systemController, version)
Srv-->>RT: Response (V2GetInfo)
RT-->>Cl: Return V2 GetInfo response
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. If you are seeing this consistently, please check "Code review limits" under "Moderation" settings.
Actionable comments posted: 6
🧹 Nitpick comments (17)
internal/storage/ledger/logs.go (1)
25-25: Improved memory efficiency with byte slice typeChanged the Memento field type from
RawMessageto[]byte, which better aligns with how the data is marshaled and stored. This should provide a more direct representation without the need for extra conversions or overhead.pkg/client/README.md (1)
1-461: Formatting inconsistency in Markdown fileThe markdown file contains hard tabs throughout the code examples rather than spaces, which may cause rendering issues in some markdown viewers.
Consider converting hard tabs to spaces throughout the markdown file for more consistent rendering across different platforms and viewers.
🧰 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: Possible missing comma found.
Context: ...ilt-innet/httpclient satisfies this interface and a default client based on the built...(AI_HYDRA_LEO_MISSING_COMMA)
[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)
[uncategorized] ~452-~452: Possible missing comma found.
Context: ...same version each time without breaking changes unless you are intentionally looking fo...(AI_HYDRA_LEO_MISSING_COMMA)
[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)
pkg/client/v2.go (1)
226-227: Check the duplicate OAuth2 scope entry.Similar to the
GetInfomethod, there appears to be a duplicate"ledger:read"entry in the OAuth2 scopes array.- OAuth2Scopes: []string{"ledger:read", "ledger:read"}, + OAuth2Scopes: []string{"ledger:read"},internal/storage/bucket/default_bucket.go (8)
95-97: Helpful reminder regarding migration order.
These inline comments underscore the importance of carefully synchronizing migration steps to avert deadlocks. Ensure thorough testing under concurrent scenarios to confirm no unintended blocking or race conditions.
144-155: Guard against redundant trigger creation.
If migrations run repeatedly, thetransaction_set_addresses_{{.ID}}trigger might be re-declared. Consider adding "IF NOT EXISTS" to the trigger creation statement, if supported.
158-170: Validate address segments trigger.
Ensure thatset_transaction_addresses_segments()procedure is defined and tested for edge-case data (e.g., empty or malformed addresses) to avoid unexpected errors or partial updates.
204-215: Confirm address array logic for accounts.
Theset_address_array_for_account()procedure must handle concurrency gracefully when multiple accounts are inserted simultaneously. Review for potential conflicts or performance overhead.
219-230: Review concurrency around setting effective volumes.
When multiple moves insert concurrently, ensureset_effective_volumes()does not inadvertently cause deadlocks or conflict with other triggers referencing the same rows.
234-245: Double-check procedure existence for updating effective volumes.
Ifupdate_effective_volumes()is absent or renamed, the trigger creation will fail. Confirm the procedure name aligns with your reference.
249-256: Ensure admin privileges for sequence creation.
Creating a new log ID sequence requires appropriate permissions. If run by a restricted user, this step could fail in certain environments. Validate deployment permissions.
261-272: Review log hash trigger correctness.
If hashing logic fails or is misconfigured, logs could remain unverified. Confirm thatset_log_hash()is robust and that partial insertions are either rolled back or handled gracefully.internal/storage/ledger/legacy/adapters.go (6)
76-78: Panic usage indicates it's not expected to be called.
Consider providing a more descriptive panic message if this method is ever incorrectly invoked.
80-82: Same approach with the unused Count method.
If there's even a slight chance of it being invoked, add a descriptive message or handle gracefully.
157-181: Constructing new big.Int might be clearer with a direct Set call.
Usingnew(big.Int).Add(new(big.Int), balance)does work, but is a bit unintuitive.Consider:
- Input: new(big.Int).Add(new(big.Int), balance), + tmp := new(big.Int) + tmp.Set(balance) + Input: tmp,
183-185: Panic for unimplemented Count is consistent with logs.
As a safety measure, a descriptive message might help if this is ever called.
208-210: Panic in GetOne clarifies unused path.
A specialized error message may further help with debugging if this is ever triggered.
212-214: Count method not implemented.
Same recommendation: provide a clearer panic message if invoked unexpectedly.
🛑 Comments failed to post (6)
internal/storage/bucket/migrations/11-make-stateless/up.sql (1)
464-464:
⚠️ Potential issueFixed character encoding for hash calculation.
Changed the encoding from 'LATIN1' to 'UTF-8' in the
set_log_hashfunction, which addresses the PR objective of fixing issues with committing transactions containing special characters in metadata. UTF-8 supports a broader range of characters and is the standard encoding for multilingual text.This change is essential as it resolves potential data corruption or hash mismatches when transaction metadata contains non-ASCII characters.
internal/storage/driver/main_test.go (1)
40-40: 💡 Verification agent
🧩 Analysis chain
Verify migration robustness
The test uses
systemstore.Migrate()which appears to be part of the migration fixes. Consider adding specific test cases that validate migration success with non-superuser privileges to directly address the Azure Postgres issue mentioned in the PR.
🏁 Script executed:
#!/bin/bash # Look for migration-related tests or scripts rg -A 5 "Migrate" --type go | grep -A 5 "test\|Test"Length of output: 9226
Action: Add explicit tests for migration under non-superuser privileges
The current test in
internal/storage/driver/main_test.gosuccessfully asserts thatsystemstore.Migrate(logging.TestingContext(), db)executes without error, confirming the migration fixes. However, the test does not validate behavior when the migration is executed with non-superuser privileges—a scenario known to impact Azure Postgres environments. Please add specific test cases that simulate a non-superuser context to ensure the migration process remains robust under these conditions.internal/api/v2/controllers_stats_test.go (1)
20-20: 💡 Verification agent
🧩 Analysis chain
Updated router initialization with environment parameter
The addition of the "develop" parameter to the NewRouter function call follows a consistent pattern of changes across test files to support environment-specific configuration. This change helps standardize router initialization across tests.
🏁 Script executed:
#!/bin/bash # Verify the NewRouter function signature has been updated to accept the new environment parameter rg -A 2 "func NewRouter\(" --type goLength of output: 533
Router Initialization Parameter Mismatch – Fix Required
The test update in internal/api/v2/controllers_stats_test.go calls
NewRouterwith two extra parameters ("develop"and a debug flag), but our verification shows that all implementations in internal/api/v2/routes.go, internal/api/v1/routes.go, and internal/api/router.go accept only thesystemControllerandauthenticatorparameters. Please update the function signatures to include the new environment-specific parameters (or adjust the test call) so that the implementations and tests remain consistent.test/e2e/api_ledgers_import_test.go (1)
52-107: 🛠️ Refactor suggestion
This test case verifies importing data from v2.1, but comparison logic can be improved.
The test successfully verifies importing logs from version 2.1 format, which aligns with the PR objective of addressing compatibility issues. However, there's a logical issue in the test implementation.
At lines 71-81 and 76-86, the test is comparing logs from the same ledger (both use
createLedgerRequest.Ledger). This doesn't actually validate what we intend to test - that the imported data matches the original source.The test should either:
- Create two different ledgers and import from one to another, or
- Verify the imported data against expected values from the import JSON
- logsFromOriginalLedger, err := ListLogs(ctx, testServer.GetValue(), operations.V2ListLogsRequest{ - Ledger: createLedgerRequest.Ledger, - }) - Expect(err).To(Succeed()) - - logsFromNewLedger, err := ListLogs(ctx, testServer.GetValue(), operations.V2ListLogsRequest{ - Ledger: createLedgerRequest.Ledger, - }) + // Verify imported logs against expected values + logsFromNewLedger, err := ListLogs(ctx, testServer.GetValue(), operations.V2ListLogsRequest{ + Ledger: createLedgerRequest.Ledger, + })Similarly for transactions and accounts comparisons that follow.
Committable suggestion skipped: line range outside the PR's diff.
pkg/client/v2.go (1)
35-36: 💡 Verification agent
🧩 Analysis chain
Verify the OAuth2 scope duplication.
The OAuth2 scopes array contains a duplicate entry:
"ledger:read"is listed twice. This is likely a typo, as other methods in this file have two different scopes (e.g.,"ledger:read"and"ledger:write").
🏁 Script executed:
#!/bin/bash # Check if other methods have duplicate OAuth2 scopes grep -n "OAuth2Scopes:" pkg/client/v2.go | head -n 15Length of output: 997
Action Required: Replace Duplicate OAuth2 Scope Entries
The current implementation across multiple methods (e.g., lines 35, 226, 405, 588, 1295, 1665, 1846, 2035, and 2568 in
pkg/client/v2.go) contains duplicated OAuth2 scopes (i.e.,[]string{"ledger:read", "ledger:read"}). In contrast, several methods correctly define both read and write scopes (e.g.,[]string{"ledger:read", "ledger:write"}at lines 767, 941, 1127, 1474, 2218, and 2399). This inconsistency suggests a copy-paste error.Recommended Actions:
- Review all occurrences where the scopes array contains duplicate
"ledger:read"entries.- Update these instances to match the intended permission set—likely changing the second
"ledger:read"to"ledger:write"where appropriate.- Verify that all methods requiring dual scopes accurately reflect the proper read/write configuration.
internal/storage/ledger/legacy/adapters.go (1)
103-120: 💡 Verification agent
🧩 Analysis chain
Check for possible panics with integer casting.
The castid = value.(int)might need a safety check if any non-int values are possible.
🏁 Script executed:
#!/bin/bash rg --context 5 -e "transactionsPaginatedResourceAdapter" -e "GetOne"Length of output: 44631
Ensure Safe Type Assertion in GetOne to Prevent Panics
It would be safer to modify the type assertion foridintransactionsPaginatedResourceAdapter.GetOneto guard against unexpected non‑int values. Although current usage (e.g. viaquery.Match("id", …)) consistently supplies an integer value, a defensive check (or a documented invariant) is advisable. For example, you might update the code as follows:func (p transactionsPaginatedResourceAdapter) GetOne(ctx context.Context, query ledgercontroller.ResourceQuery[any]) (*ledger.Transaction, error) { var id int _ = query.Builder.Walk(func(_ string, _ string, value any) error { intVal, ok := value.(int) if !ok { return fmt.Errorf("expected int for id, got %T", value) } id = intVal return nil }) return p.store.GetTransactionWithVolumes(ctx, GetTransactionQuery{ PITFilterWithVolumes: PITFilterWithVolumes{ PITFilter: PITFilter{ PIT: query.PIT, OOT: query.OOT, }, ExpandVolumes: slices.Contains(query.Expand, "volumes"), ExpandEffectiveVolumes: slices.Contains(query.Expand, "effectiveVolumes"), }, ID: id, }) }This update prevents runtime panics if an unexpected non‑int value is encountered and documents the expectation about the type of
id.
gfyrag
force-pushed
the
hotfix/v2.2/ledger-creations-atomic
branch
from
6e03ca9 to
4ec427b
Compare
No description provided.