Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant Client as SDK Client
participant Action as Action Handler
participant ContractAPI as ContractAPI Layer
participant Decoder as DecodeCallResult
Client->>Client: LoadActions()
Client->>Action: GetHistory(ctx, input)
Action->>Action: Validate BridgeIdentifier & Wallet
Action->>Action: Apply defaults (limit:20, offset:0)
Action->>Action: Build actionName & args
Action->>ContractAPI: call(ctx, actionName, args)
ContractAPI->>ContractAPI: Execute contract method
ContractAPI-->>Action: Return raw result
Action->>Decoder: DecodeCallResult(result)
Decoder->>Decoder: Handle pointer fields & nil values
Decoder-->>Action: []BridgeHistory
Action-->>Client: []BridgeHistory
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@core/contractsapi/bridge_history.go`:
- Around line 19-26: Validate and bound-check user-supplied paging values before
using them: when reading input.Limit and input.Offset, ensure they are
non-nil-checked and that their values are not negative (return a bad-request
error if negative), set a sensible default for limit (keep limit := 20) and cap
limit to a maximum (e.g., maxLimit := 100) before assigning to the local
variables `limit` and `offset`; use `input.Limit`, `input.Offset`, and the local
`limit`/`offset` variables as the change points and add a constant like
`maxLimit` or reuse an existing pagination config.
In `@core/types/bridge_types.go`:
- Around line 18-23: GetHistoryInput's Limit and Offset lack validation so
negative values could slip through; update the struct tags on GetHistoryInput
(fields Limit and Offset) to include validation constraints (e.g., for Limit use
`validate:"omitempty,min=1"` to require positive limits and for Offset use
`validate:"omitempty,min=0"` to forbid negatives), and ensure the validation
library is invoked where GetHistoryInput is validated so these constraints are
enforced.
In `@core/types/stream.go`:
- Around line 127-129: GetHistory is a bridge-specific operation that breaks
IAction cohesion and forces non-bridge implementors (e.g., TransportAction) to
stub it; refactor by moving GetHistory off IAction into a new IBridgeAction
interface (or exposing it directly on Client/LoadActions as IBridgeAction) and
update implementations to implement IBridgeAction instead of adding a stub for
GetHistory; keep existing pattern in mind for
BatchStreamExists/BatchFilterStreamsByExistence so callers remain compatible and
update any factory/LoadActions wiring to return the new interface where needed.
In `@docs/api-reference.md`:
- Around line 2049-2066: The BridgeHistory struct documentation is incorrectly
located inside the "Attestation Actions Interface > Types" section; move the
entire BridgeHistory definition out of that attestation block and place it
either in a new "Bridge History Types" section or under the GetHistory method
documentation (reference symbol: BridgeHistory and GetHistory) so readers no
longer associate it with attestation; ensure you remove the original occurrence
and update any nearby cross-references or table-of-contents anchors that pointed
to the old location.
In `@examples/history_example/main.go`:
- Around line 35-37: The comment on the endpoint default is misleading: update
the comment next to the endpoint variable assignment (the line setting endpoint
= "https://gateway.testnet.truf.network") to accurately state that it defaults
to the testnet gateway (e.g., "Default to testnet gateway") or remove the "local
node" wording so it reflects the true URL.
- Around line 90-101: The helper formatHexShort is being redeclared inside the
loop and the surrounding draft comments are noise; move the formatHexShort
function definition out of the loop (declare it once above the loop that prints
history) and remove the draft/comment lines (those "Shorten hashes..." and "Or
shortening..." notes) so the loop only contains the printing logic and calls to
formatHexShort.
🧹 Nitpick comments (4)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@core/contractsapi/bridge_history.go`: - Around line 19-26: Validate and bound-check user-supplied paging values before using them: when reading input.Limit and input.Offset, ensure they are non-nil-checked and that their values are not negative (return a bad-request error if negative), set a sensible default for limit (keep limit := 20) and cap limit to a maximum (e.g., maxLimit := 100) before assigning to the local variables `limit` and `offset`; use `input.Limit`, `input.Offset`, and the local `limit`/`offset` variables as the change points and add a constant like `maxLimit` or reuse an existing pagination config. In `@core/types/bridge_types.go`: - Around line 18-23: GetHistoryInput's Limit and Offset lack validation so negative values could slip through; update the struct tags on GetHistoryInput (fields Limit and Offset) to include validation constraints (e.g., for Limit use `validate:"omitempty,min=1"` to require positive limits and for Offset use `validate:"omitempty,min=0"` to forbid negatives), and ensure the validation library is invoked where GetHistoryInput is validated so these constraints are enforced. In `@core/types/stream.go`: - Around line 127-129: GetHistory is a bridge-specific operation that breaks IAction cohesion and forces non-bridge implementors (e.g., TransportAction) to stub it; refactor by moving GetHistory off IAction into a new IBridgeAction interface (or exposing it directly on Client/LoadActions as IBridgeAction) and update implementations to implement IBridgeAction instead of adding a stub for GetHistory; keep existing pattern in mind for BatchStreamExists/BatchFilterStreamsByExistence so callers remain compatible and update any factory/LoadActions wiring to return the new interface where needed. In `@examples/history_example/main.go`: - Around line 90-101: The helper formatHexShort is being redeclared inside the loop and the surrounding draft comments are noise; move the formatHexShort function definition out of the loop (declare it once above the loop that prints history) and remove the draft/comment lines (those "Shorten hashes..." and "Or shortening..." notes) so the loop only contains the printing logic and calls to formatHexShort.core/types/stream.go (1)
127-129: Consider whetherGetHistorybelongs onIActionor a dedicated interface.
IActioncurrently groups stream-related operations (records, indexes, visibility, permissions).GetHistoryis a bridge-specific operation that doesn't conceptually relate to stream data. A separateIBridgeActioninterface (or placing it on theClientdirectly without going throughLoadActions()) would preserve interface cohesion and avoid forcing allIActionimplementors (includingTransportAction) to stub this method.That said, this follows the same pattern as
BatchStreamExists/BatchFilterStreamsByExistencewhich are also not strictly per-stream operations, so this may be an intentional architectural choice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/types/stream.go` around lines 127 - 129, GetHistory is a bridge-specific operation that breaks IAction cohesion and forces non-bridge implementors (e.g., TransportAction) to stub it; refactor by moving GetHistory off IAction into a new IBridgeAction interface (or exposing it directly on Client/LoadActions as IBridgeAction) and update implementations to implement IBridgeAction instead of adding a stub for GetHistory; keep existing pattern in mind for BatchStreamExists/BatchFilterStreamsByExistence so callers remain compatible and update any factory/LoadActions wiring to return the new interface where needed.core/types/bridge_types.go (1)
18-23: Consider adding validation constraints onLimitandOffset.Negative values for
LimitorOffsetcould produce unexpected query behavior. You could addvalidate:"omitempty,min=0"(ormin=1for Limit) to catch invalid input early on the client side, consistent with how validation is already used on the required fields.♻️ Optional: Add validation constraints
type GetHistoryInput struct { BridgeIdentifier string `validate:"required"` Wallet string `validate:"required"` - Limit *int - Offset *int + Limit *int `validate:"omitempty,min=1"` + Offset *int `validate:"omitempty,min=0"` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/types/bridge_types.go` around lines 18 - 23, GetHistoryInput's Limit and Offset lack validation so negative values could slip through; update the struct tags on GetHistoryInput (fields Limit and Offset) to include validation constraints (e.g., for Limit use `validate:"omitempty,min=1"` to require positive limits and for Offset use `validate:"omitempty,min=0"` to forbid negatives), and ensure the validation library is invoked where GetHistoryInput is validated so these constraints are enforced.core/contractsapi/bridge_history.go (1)
19-26: No validation onLimitandOffsetvalues.User-supplied
*input.Limitand*input.Offsetare used directly without bounds checking. A negative limit or offset would be passed straight to the backend, which may produce unexpected results or errors that are harder to diagnose.Proposed fix
limit := 20 if input.Limit != nil { + if *input.Limit < 0 { + return nil, errors.New("limit must be non-negative") + } limit = *input.Limit } offset := 0 if input.Offset != nil { + if *input.Offset < 0 { + return nil, errors.New("offset must be non-negative") + } offset = *input.Offset }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/contractsapi/bridge_history.go` around lines 19 - 26, Validate and bound-check user-supplied paging values before using them: when reading input.Limit and input.Offset, ensure they are non-nil-checked and that their values are not negative (return a bad-request error if negative), set a sensible default for limit (keep limit := 20) and cap limit to a maximum (e.g., maxLimit := 100) before assigning to the local variables `limit` and `offset`; use `input.Limit`, `input.Offset`, and the local `limit`/`offset` variables as the change points and add a constant like `maxLimit` or reuse an existing pagination config.examples/history_example/main.go (1)
90-101: MoveformatHexShortout of the loop and remove draft comments.The helper is re-declared on every iteration, and lines 90-92 read like leftover notes rather than documentation.
Proposed cleanup
Define
formatHexShortonce before the loop and drop the draft comments:+ formatHexShort := func(b []byte) string { + if len(b) == 0 { + return "null" + } + if len(b) > 4 { + return fmt.Sprintf("0x%x...", b[:4]) + } + return fmt.Sprintf("0x%x", b) + } + for _, rec := range history { // Format timestamp tm := time.Unix(rec.BlockTimestamp, 0) timeStr := tm.Format(time.RFC3339) - // Shorten hashes for display if needed, but printing full for now as per request - // Or shortening to keep table readable? The CLI output truncated them. - // "0x%x..." logic was used before. I will use a helper to optionally shorten. - formatHexShort := func(b []byte) string { - if len(b) == 0 { - return "null" - } - if len(b) > 4 { - return fmt.Sprintf("0x%x...", b[:4]) - } - return fmt.Sprintf("0x%x", b) - } - // Handle nullable external block height🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/history_example/main.go` around lines 90 - 101, The helper formatHexShort is being redeclared inside the loop and the surrounding draft comments are noise; move the formatHexShort function definition out of the loop (declare it once above the loop that prints history) and remove the draft/comment lines (those "Shorten hashes..." and "Or shortening..." notes) so the loop only contains the printing logic and calls to formatHexShort.
resolves: https://github.com/truflation/website/issues/3313
Summary by CodeRabbit
New Features
Documentation