feat: Automated Release Workflow & Prevent Duplicate Transactions #55
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds CI release and dev-release workflows and a GoReleaser config; introduces a MemoCache (*utils.Cache) initialized in main; adds SetNX/Delete to the cache; and enforces memo-based in-flight locking and duplicate-memo checks in the /api/send handler. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub
participant GHA as GitHub Actions
participant GR as GoReleaser
participant TG as Telegram
Dev->>GH: Merge PR to main
GH->>GHA: Trigger release workflow
activate GHA
GHA->>GHA: Checkout repo (fetch-depth:0)
GHA->>GHA: Setup Go 1.21
GHA->>GHA: Bump version & create tag
GHA->>GR: Run GoReleaser (build & publish)
activate GR
GR-->>GHA: Artifacts published
deactivate GR
alt Release success
GHA->>TG: Send success message (version, PR title, author, release link)
TG-->>GHA: Ack
end
deactivate GHA
sequenceDiagram
participant Client as Client
participant API as API Service
participant Cache as MemoCache
participant DB as Database
Client->>API: POST /api/send (memo X)
API->>Cache: SetNX "memo:X"
alt SetNX returns false
Cache-->>API: lock exists
API-->>Client: 409 "transaction already processing"
else SetNX true
Cache-->>API: lock acquired
API->>DB: Query completed transactions WHERE memo LIKE '%X'
alt Completed found
DB-->>API: existing record
API->>Cache: Delete "memo:X"
API-->>Client: 409 "memo already used"
else None found
API->>API: proceed with send flow (payment processing, DB writes)
API->>Cache: Delete "memo:X" (on completion or error)
API-->>Client: 200 / appropriate response
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
49-60: Consider adding error handling to curl commands.The Telegram notification curl commands don't check HTTP response codes. Consider adding the
-fflag to fail on HTTP error responses, or check the response explicitly, to ensure failures are surfaced rather than silently masked.Apply this diff to add error handling:
- name: Notify Telegram Success if: success() run: | - curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ + curl -sf -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="${{ env.CHAT_ID }}" \ -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${PR_TITLE}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ -d parse_mode="Markdown" - name: Notify Telegram Failure if: failure() run: | - curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ + curl -sf -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="${{ env.CHAT_ID }}" \ - -d text="❌ *Release Failed!*%0A%0AThe release workflow failed for PR: ${PR_TITLE}" \ + -d text="❌ *Release Failed!*%0A%0AThe release workflow failed for PR: ${PR_TITLE}" \ -d parse_mode="Markdown"The
-fflag causes curl to return a non-zero exit code on HTTP errors (4xx/5xx), which will fail the step if the Telegram API returns an error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks).goreleaser.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release.yml
26-26: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
48-48: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
56-56: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ 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). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
.goreleaser.yaml (1)
1-24: GoReleaser configuration is well-structured.The build configuration appropriately enables CGO for C bindings, targets linux/amd64, and uses standard flags for static compilation. The linker flags (-s -w) correctly strip symbols to minimize binary size. Changelog filtering excludes non-feature commits as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/api/send.go (1)
252-288: Consider: Admin approval flow releases memo lock immediately.When a transaction requires admin approval, the function returns at line 287, triggering the deferred
Delete(memoLockKey)and releasing the memo lock. This allows subsequent requests with the same memo to create additional pending transactions and approval requests before the first is approved or rejected.If the intent is to prevent duplicate approval requests for the same memo, the lock should remain until the transaction is actually approved and executed (or rejected). This would require a different locking strategy, such as:
- Storing the pending transaction ID in the cache instead of "locked"
- Checking for pending transactions with the same memo at the start
- Releasing the lock only after approval/rejection
Is this intended behavior, or should duplicate approval requests with the same memo be prevented?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/api/lightning.go(1 hunks)internal/api/send.go(2 hunks)internal/utils/cache.go(1 hunks)main.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
internal/utils/cache.go (1)
internal/runtime/mutex/mutex.go (2)
Lock(110-127)Unlock(130-141)
main.go (4)
internal/api/lightning.go (1)
Service(17-20)internal/api/userpage/userpage.go (1)
Service(20-22)internal/api/admin/admin.go (1)
Service(7-9)internal/utils/cache.go (1)
NewCache(19-24)
internal/api/lightning.go (2)
internal/telegram/bot.go (2)
TipBot(25-34)Cache(35-37)internal/utils/cache.go (1)
Cache(13-17)
internal/api/send.go (2)
internal/api/send_config.go (1)
GetWhitelistedWallets(8-10)internal/api/lightning.go (1)
RespondError(26-30)
⏰ 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: Analyze (go)
- GitHub Check: build-and-deploy
🔇 Additional comments (4)
internal/utils/cache.go (1)
47-51: LGTM! Delete method implementation is correct.The Delete method correctly follows the mutex locking pattern used by Set and Get, ensuring thread-safe deletion of cache entries.
main.go (1)
94-97: LGTM! MemoCache initialization is correct.The MemoCache is properly initialized with a 5-minute TTL, which is reasonable for preventing duplicate memo processing while not holding locks indefinitely.
internal/api/lightning.go (1)
18-19: LGTM! Service struct extension is appropriate.The MemoCache field is correctly added to the Service struct and properly initialized in main.go.
internal/api/send.go (1)
154-154: Consider: Lock granularity may be too coarse.The memo lock key is based solely on the memo text, meaning all API sends with the same memo (e.g., "lunch", "coffee") are blocked regardless of sender or recipient. If this is intended for global memo uniqueness, this is correct. However, if the intent is to prevent duplicate sends from the same wallet, consider including the wallet ID:
-memoLockKey := fmt.Sprintf("api_send_memo_%s", req.Memo) +memoLockKey := fmt.Sprintf("api_send_memo_%s_%s", walletID, req.Memo)This would allow different wallets to use the same memo concurrently.
Please clarify the intended scope: Should memo locking be global (current behavior) or per-wallet?
xbuddhi
changed the title
helloscoopa
left a comment
helloscoopa
left a comment
There was a problem hiding this comment.
Issue raised by coderabbit needs to be taken care of, apart from that the implementation looks good:
Implementation Quality:
- Thread-safe operations throughout
- Proper error handling and logging
- Fail-closed security posture
- Multiple layers of protection (in-flight lock + DB check + TTL)
- Clean separation of concerns
…_RELEASE_CHAT_IDS) and support threads in Telegram notifications
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
26-28: Updateactions/setup-gofrom v4 to v6 for compatibility with current runners.The workflow uses
actions/setup-go@v4, which is outdated. Update to v6 to receive security patches and current Go version support.uses: actions/setup-go@v6
48-66: Fix script injection vulnerability: use environment variable forgithub.event.pull_request.title.
github.event.pull_request.titleis user-controlled and passed directly into the inline script on lines 58 and 63. Per GitHub's security guidelines, this should be passed through an environment variable to prevent command injection attacks.Apply this diff to secure the notification step:
jobs: release: if: github.event.pull_request.merged == true runs-on: ubuntu-latest env: BOT_TOKEN: ${{ secrets.MAIN_BOT_TOKEN }} CHAT_ID: ${{ vars.RELEASE_CHAT_IDS }} + PR_TITLE: ${{ github.event.pull_request.title }} steps: - name: Checkout uses: actions/checkout@v4 with: fetch-depth: 0 - name: Set up Go uses: actions/setup-go@v4 with: go-version: '1.21' - name: Bump version and push tag id: tag_version uses: mathieudutour/github-tag-action@v6.1 with: github_token: ${{ secrets.GITHUB_TOKEN }} default_bump: patch - name: Run GoReleaser uses: goreleaser/goreleaser-action@v5 with: distribution: goreleaser version: latest args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Notify Telegram Success if: success() run: | IFS=',' read -ra TARGETS <<< "${{ env.CHAT_ID }}" for target in "${TARGETS[@]}"; do target=$(echo "$target" | xargs) if [[ "$target" == *":"* ]]; then chat_id=${target%%:*} thread_id=${target#*:} curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="$chat_id" \ -d message_thread_id="$thread_id" \ - -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${{ github.event.pull_request.title }}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ + -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${PR_TITLE}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ -d parse_mode="Markdown" else curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="$target" \ - -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${{ github.event.pull_request.title }}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ + -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${PR_TITLE}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ -d parse_mode="Markdown" fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release-dev.yml(1 hunks).github/workflows/release.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release.yml
26-26: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
48-48: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
.github/workflows/release-dev.yml
24-24: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (3)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (actions)
- GitHub Check: build-and-deploy
🔇 Additional comments (1)
.github/workflows/release-dev.yml (1)
50-73: Telegram notification logic is well-designed and secure.The comma-separated chat ID parsing with optional thread ID support (via
chat_id:thread_idformat) is correct and handles edge cases properly. The notification message is appropriately formatted and does not use untrusted inputs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/utils/cache.go (1)
47-68: Delete/SetNX implementations are concurrency-safe and match existing TTL semantics
DeleteandSetNXcorrectly reuse the cache’s mutex, andSetNXprovides the needed atomic “set if not exists or expired” behavior for memo locking. This is a solid fix for the earlier race condition risk.If you want to reduce duplication later, you could have
Setdelegate toSetNX(or vice versa) so the TTL/assignment logic lives in one place, but it’s not required for correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/api/send.go(2 hunks)internal/utils/cache.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
internal/api/send.go (4)
internal/api/send_config.go (1)
GetWhitelistedWallets(8-10)internal/api/lightning.go (1)
RespondError(26-30)internal/storage/bunt.go (1)
DB(16-18)internal/telegram/transaction.go (1)
Transaction(13-34)
internal/utils/cache.go (1)
internal/runtime/mutex/mutex.go (2)
Lock(110-127)Unlock(130-141)
⏰ 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: Analyze (go)
- GitHub Check: build-and-deploy
🔇 Additional comments (2)
internal/api/send.go (2)
121-129: Authenticated wallet extraction and whitelist lookup look goodCasting the authenticated wallet from context to
stringand validating it againstGetWhitelistedWallets()is straightforward and consistent with the surrounding flow; no issues here.
153-183: Memo-based locking and duplicate check correctly fix the raceThe memo block successfully implements both protections:
- Uses
SetNXonMemoCacheto atomically acquire a per-memo lock; concurrent requests with the same memo fail fast with "already processing".- Deletes the memo lock via
defer s.MemoCache.Delete(memoLockKey)on all return paths from this handler.- Adds a DB-level guard (
LIKEon the memo suffix withsuccess = true) to reject non-concurrent repeat calls as "already completed".This removes the previous TOCTOU window and provides both "ongoing" and "already completed" protection.
MemoCacheis properly initialized inmain.goasutils.NewCache(time.Minute * 5)with no nil pointer risk.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/release.yml (2)
26-28: Updateactions/setup-gofrom v4 to v6.1.0.The workflow pins an outdated action version. Update to v6 (latest is v6.1.0) to receive security patches and ensure compatibility with current Go tooling.
- name: Set up Go uses: actions/setup-go@v6 with: go-version: '1.21'
58-68: Fix script injection vulnerability: Pass PR title via environment variable.
github.event.pull_request.titleis user-controlled and should not be interpolated directly into inline scripts. This creates a command injection attack vector. Pass it through an environment variable instead, as documented in GitHub's security guidelines.jobs: release: if: github.event.pull_request.merged == true runs-on: ubuntu-latest env: BOT_TOKEN: ${{ secrets.MAIN_BOT_TOKEN }} CHAT_ID: ${{ vars.RELEASE_CHAT_IDS }} + PR_TITLE: ${{ github.event.pull_request.title }} steps: ... - name: Notify Telegram Success if: success() run: | IFS=',' read -ra TARGETS <<< "${{ env.CHAT_ID }}" for target in "${TARGETS[@]}"; do target=$(echo "$target" | xargs) if [[ "$target" == *":"* ]]; then chat_id=${target%%:*} thread_id=${target#*:} curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="$chat_id" \ -d message_thread_id="$thread_id" \ - -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${{ github.event.pull_request.title }}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ + -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${PR_TITLE}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ -d parse_mode="Markdown" else curl -s -X POST https://api.telegram.org/bot${{ env.BOT_TOKEN }}/sendMessage \ -d chat_id="$target" \ - -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${{ github.event.pull_request.title }}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ + -d text="🚀 *New Release Published!*%0A%0A*Version:* ${{ steps.tag_version.outputs.new_tag }}%0A*Title:* ${PR_TITLE}%0A*Author:* ${{ github.actor }}%0A%0A[View Release](https://github.com/${{ github.repository }}/releases/tag/${{ steps.tag_version.outputs.new_tag }})" \ -d parse_mode="Markdown" fi done.github/workflows/release-dev.yml (1)
23-26: Updateactions/setup-gofrom v4 to v6.1.0.The workflow pins an outdated action version. Update to v6 (latest is v6.1.0) to receive security patches and ensure compatibility with current Go tooling.
- name: Set up Go uses: actions/setup-go@v6 with: go-version: '1.21'
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
49-69: Add guard for empty CHAT_ID targets to prevent unnecessary curl calls.The dev workflow includes a guard (
if [[ -z "$target" ]]) to skip empty targets after trimming whitespace. Add the same safeguard here for robustness.- name: Notify Telegram Success if: success() run: | IFS=',' read -ra TARGETS <<< "${{ env.CHAT_ID }}" for target in "${TARGETS[@]}"; do target=$(echo "$target" | xargs) + if [[ -z "$target" ]]; then + continue + fi if [[ "$target" == *":"* ]]; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release-dev.yml(1 hunks).github/workflows/release.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release-dev.yml
24-24: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/release.yml
26-26: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
51-51: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ 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: Analyze (go)
- GitHub Check: build-and-deploy
🔇 Additional comments (1)
.github/workflows/release-dev.yml (1)
61-84: Telegram notification logic is sound.The dev release workflow correctly:
- Parses comma-separated CHAT_IDs with optional thread_id support
- Trims whitespace and skips empty targets (lines 67-69)
- Uses commit SHA and tag (not user-controlled PR title), avoiding injection risks
- Sends formatted Markdown notifications to all targets
This is a solid implementation for multi-target notifications.
feat: Automated Release Workflow & Prevent Duplicate Transactions
🚀 Summary
This PR introduces a fully automated release pipeline using GitHub Actions and GoReleaser, AND implements a robust locking mechanism for the API send endpoint to prevent duplicate transactions.
✨ Key Changes
1. Prevent Duplicate Transactions (New)
MemoCacheto see if a transaction with the provided memo is currently processing. Returns "Transaction ongoing" if found.Deletemethod toCachestruct and initializedMemoCacheinmain.go.2. Automated Release Workflow
New Workflow (.github/workflows/release.yml):
main.mathieudutour/github-tag-actionto automatically bump the patch version.GoReleaserto build the binary and create a GitHub Release.MAIN_BOT_TOKENsecret for the bot token.RELEASE_CHAT_IDSsecret for chat IDs.RELEASE_CHAT_IDSsupports multiple comma-separated chat IDs, optionally with thread IDs (e.g.,-100123,-100456:7).GoReleaser Config (.goreleaser.yaml):
CGO_ENABLED=1.-s -w).🧪 Testing
pull_request(closed + merged).chat_id:thread_id).Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.