Conversation
WalkthroughThe pull request introduces changes to the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
receipt/handler.go(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Deploy to ECS
receipt/handler.go
[error] 134-134: undefined: pb.ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERT
🔇 Additional comments (1)
receipt/handler.go (1)
Line range hint
151-162: LGTM! Clear status handling for reverted transactionsThe implementation correctly updates the
Reasonfield in the receipt to provide more specific information about on-chain transaction reverts, which aligns well with the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
receipt/handler.go (1)
131-135:⚠️ Potential issueAdd nil check for receipt before accessing Status field
The receipt should be checked for nil before accessing its Status field to prevent potential panic.
reason := status.Status.String() -if receipt.Status == types.ReceiptStatusFailed { +if receipt != nil && receipt.Status == types.ReceiptStatusFailed { reason = pb.ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERT.String() }
🧹 Nitpick comments (1)
receipt/handler.go (1)
131-139: Consider consolidating the revert status conditionsThe code sets the same revert status in two separate conditions. Consider consolidating them to make the logic clearer and prevent potential confusion about the final state.
reason := status.Status.String() -if receipt.Status == types.ReceiptStatusFailed { - reason = pb.ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERT.String() -} - -if !it.Event.Success { +if (receipt != nil && receipt.Status == types.ReceiptStatusFailed) || !it.Event.Success { reason = pb.ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERT.String() -} }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(4 hunks)receipt/handler.go(3 hunks)
🔇 Additional comments (3)
go.mod (1)
8-8: Verify model dependency version compatibilityThe update to
github.com/blndgs/model v0.56.0is significant. Please ensure this version includes the requiredProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERTenum from PR blndgs/model#61.✅ Verification successful
✓ Model dependency version compatibility verified
The required
ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERTenum is present and actively used in the codebase with model v0.56.0, as evidenced by its usage inreceipt/handler.go.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ProcessingStatus enum is properly defined in the model repository # Test: Search for the ProcessingStatus enum definition ast-grep --pattern 'enum ProcessingStatus { $$$ PROCESSING_STATUS_ON_CHAIN_REVERT = $_ $$$ }'Length of output: 101
Script:
#!/bin/bash # Search for ProcessingStatus in .proto files echo "=== Searching .proto files ===" fd -e proto -x rg -l "ProcessingStatus" echo -e "\n=== Searching for ProcessingStatus enum usage ===" rg "ProcessingStatus_PROCESSING_STATUS_ON_CHAIN_REVERT" echo -e "\n=== Checking imports of model package ===" rg "github.com/blndgs/model"Length of output: 1926
receipt/handler.go (2)
17-18: LGTM!The protobuf import is correctly added and follows the package path convention.
155-155: LGTM!The UserOperationReceipt struct correctly uses the newly determined reason value.
hpsing
left a comment
hpsing
left a comment
There was a problem hiding this comment.
we should be using updateStatus from the store so that it can be queried using getReceipt with currect status.
* fix: userops status fixes * feat: custom user receipt * fix: receipt fetch from db * fix: status return for cases * fix: suggested changes * fix: store package for db activities * fix: hardcoded lines * Update status if tx reverts onchain (#72) * check if tx reverted onchain * use success too for checks * move check to a single branch * store in badger --------- Co-authored-by: singhhp1069 <singhhp1069@gmail.com>
* fix: userops status fixes * feat: custom user receipt * fix: receipt fetch from db * fix: status return for cases * fix: suggested changes * fix: store package for db activities * fix: hardcoded lines * Update status if tx reverts onchain (#72) * check if tx reverted onchain * use success too for checks * move check to a single branch * store in badger --------- Co-authored-by: singhhp1069 <singhhp1069@gmail.com>
3 participants
Check the receipt status and if it fails, use a
reasonthat specifically tells you it is onchain but revertedNeeds blndgs/model#61
Summary by CodeRabbit
Bug Fixes
Chores