services/horizon: Return both inner and outer result codes for fee bump transactions. #4081
Conversation
This file contains 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
leighmcculloch
approved these changes
Nov 16, 2021
|
@tamirms PTAL. I am not sure the code is correct for a scenario in which a transaction and a fee bump transaction concurrently compete to get to the ledger, and the transaction makes it to the ledger first. |
tamirms
reviewed
Nov 17, 2021
| @@ -38,18 +38,29 @@ func (fte *FailedTransactionError) Result() (result xdr.TransactionResult, err e | |||
| return | |||
| } | |||
|
|
|||
| func (fte *FailedTransactionError) TransactionResultCode(transactionHash string) (result string, err error) { | |||
| type OuterInnerResultCodes struct { | |||
There was a problem hiding this comment.
nit: since it's an exported type it would be worth adding a comment for this struct
There was a problem hiding this comment.
I think it would be better to rename the struct to:
type ResultCodes struct {
Code string
InnerCode string
}
that way it matches the structure of TransactionResultCodes in the protocols/horizon package where we default to TransactionCode and only populate InnerTransactionCode if the request was attempting to submit a feebump transaction
erika-sdf
force-pushed
the
txn
branch
2 times, most recently
from
a7bcef9 to
8474b36
Compare
tamirms
approved these changes
Nov 18, 2021
erika-sdf
added a commit
to erika-sdf/go
that referenced
this pull request
Dec 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Return both inner and outer result codes for fee bump transactions.
Why
Currently only the outer code is returned for fee bump transactions
Known limitations
N/A