-
Notifications
You must be signed in to change notification settings - Fork 1
Add ability to search unconfirmed transactions #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds the ability to search for unconfirmed transactions by ID, implementing step 1 of Issue #291. The implementation extends the existing Search functionality to check the transaction pool when a transaction is not found in the confirmed blockchain data.
Changes:
- Modified the
Searchfunction to fall back to checking unconfirmed transaction pools (both V1 and V2) when the database search returns no results - Added a test case to verify searching for unconfirmed V1 transactions works correctly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| explorer/explorer.go | Extended Search function to query unconfirmed V1 and V2 transaction pools after database search fails |
| api/api_test.go | Added test case for searching unconfirmed V1 transactions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
611a790 to
88df3f6
Compare
explorer/explorer.go
Outdated
|
|
||
| // check unconfirmed transaction pool | ||
| var txnID types.TransactionID | ||
| if err := txnID.UnmarshalText([]byte(id)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is incorrect. The current error might be ErrNoSearchResults for an identifier that's not a transaction id. So if we fail to unmarshal it here we might be return a different, unwanted, error. We also don't take into account the txid prefix.
We should probably get rid of SearchTypeInvalid all together and in case search comes up short, optimistically parse the id as a txn id and check the pool, only in the success case we null the error and return the result, but in all other cases we return ErrNoSearchResults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We removed the prefixes from all the types in core. I changed it to not error if it fails to unmarshal. I'd rather not change the API here but I don't feel strongly either way.
#291