Skip to content

Conversation

@georgehao
Copy link
Member

@georgehao georgehao commented Nov 25, 2025

Purpose or design rationale of this PR

Describe your change. Make sure to answer these three questions: What does this PR do? Why does it do it? How does it do it?

don't need this check, because will insert a empty value to the cache when the called addresss really don't have the claimed withdrawal.

if len(txs) == 0 {
if err := pipe.ZAdd(ctx, cacheKey, &redis.Z{Score: 0, Member: "empty_page"}).Err(); err != nil {
log.Error("failed to add empty page indicator to sorted set", "error", err)
return err
}

PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

Deployment tag versioning

Has tag in common/version.go been updated or have you added bump-version label to this PR?

  • No, this PR doesn't involve a new deployment, git tag, docker image tag
  • Yes

Breaking change label

Does this PR have the breaking-change label?

  • No, this PR is not a breaking change
  • Yes

Summary by CodeRabbit

  • Bug Fixes

    • Fixed error handling when retrieving withdrawal history with no results; now processes empty datasets appropriately instead of terminating with an error.
  • Chores

    • Updated version to v4.7.4.

✏️ Tip: You can customize this high-level summary in your review settings.

@georgehao georgehao requested a review from Thegaram November 25, 2025 14:00
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

The pull request updates the version from v4.7.3 to v4.7.4 and modifies error handling in the GetL2UnclaimedWithdrawalsByAddress function. Instead of returning an error when the withdrawal history is empty, the function now proceeds to cache the empty result and delegates outcome handling downstream.

Changes

Cohort / File(s) Change Summary
History logic control flow
bridge-history-api/internal/logic/history_logic.go
Removed early error return when txHistoryInfos slice is empty; empty datasets now proceed to caching layer for downstream handling
Version bump
common/version/version.go
Updated internal version tag from v4.7.3 to v4.7.4

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The version bump is a straightforward update with no logic changes
  • The history logic modification removes a single error path; verify that empty withdrawal scenarios are properly handled by downstream caching and result processing logic

Possibly related PRs

Suggested reviewers

  • Thegaram
  • yiweichi

Poem

🐰 A version hops forth, from four-seven-three,
Empty withdrawals now cached gracefully!
No early exit when history runs dry—
Downstream handling catches each reply.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'remove unused check' does not follow conventional commits format and lacks the required type prefix (build, ci, docs, feat, fix, perf, refactor, style, test). Update the title to follow conventional commits, for example: 'fix: remove unused check' or 'refactor: remove unused check' depending on the change nature.
Description check ❓ Inconclusive The PR description is incomplete. While the author selected 'fix' as the commit type, the 'Purpose or design rationale' section lacks a clear explanation answering what the PR does, why it does it, and how it does it. Provide a complete description of the changes: clearly state what was changed, why the early return check was removed, and explain the caching behavior being relied upon for empty results.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/remove_unused_check

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.54%. Comparing base (15a2347) to head (738f3c5).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1765   +/-   ##
========================================
  Coverage    36.53%   36.54%           
========================================
  Files          247      247           
  Lines        21190    21186    -4     
========================================
  Hits          7742     7742           
+ Misses       12618    12614    -4     
  Partials       830      830           
Flag Coverage Δ
bridge-history-api 8.08% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@Thegaram Thegaram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Please fill in the PR description

@georgehao georgehao merged commit de72e2d into develop Nov 25, 2025
6 checks passed
@georgehao georgehao deleted the fix/remove_unused_check branch November 25, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants