Update tx queue memo filtering #15
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR updates the memo filtering logic for Soroban transactions by removing the special case handling for source-account-only authorization. The change simplifies the validation to always reject Soroban transactions that have memos or use muxed ED25519 accounts, regardless of the authorization type.
- Removes the source account authorization check that previously allowed memo usage
- Simplifies the validation logic to a single conditional check
- Updates the corresponding test to expect ADD_STATUS_ERROR instead of ADD_STATUS_PENDING
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/herder/TransactionQueue.cpp | Simplifies validateSorobanMemo function by removing source account auth logic and combining memo/muxed account checks |
| src/herder/test/TransactionQueueTests.cpp | Updates test expectation from ADD_STATUS_PENDING to ADD_STATUS_ERROR for Soroban tx with memo |
|
|
||
| REQUIRE(app->getHerder().recvTransaction(txWithMemo, false).code == | ||
| TransactionQueue::AddResultCode::ADD_STATUS_PENDING); | ||
| TransactionQueue::AddResultCode::ADD_STATUS_ERROR); |
There was a problem hiding this comment.
This test change indicates a behavior modification in transaction processing. According to the coding guidelines, fee bump tests should be written to accompany any logic changes to transaction processing. Consider adding fee bump test coverage for this memo filtering change.
6 tasks
github-merge-queue bot
pushed a commit
to stellar/stellar-core
that referenced
this pull request
Aug 26, 2025
# Description Adds [copilot instructions](https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions) to comment if fee bumps tests should be written. We could also make additional changes for more targeted reviews on things like style guidelines and missing tests. Here's an example of this in action - sisuresh#15 (comment) <!--- Describe what this pull request does, which issue it's resolving (usually applicable for code changes). ---> # Checklist - [ ] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [ ] Rebased on top of master (no merge commits) - [ ] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [ ] Compiles - [ ] Ran all tests - [ ] If change impacts performance, include supporting evidence per the [performance document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #X
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)