CAP-0015 Implementation #2419
Merged
CAP-0015 Implementation #2419
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
jonjove
force-pushed
the
fee-bump-transactions
branch
from
9861248
to
79fa78d
Compare
MonsieurNicolas
requested changes
Feb 12, 2020
MonsieurNicolas
requested changes
Feb 13, 2020
src/herder/TransactionQueue.cpp
Outdated
| { | ||
| int64_t oldFee = (*oldTxIter)->getFeeBid(); | ||
| if (fee < FEE_MULTIPLIER * oldFee) | ||
| { |
There was a problem hiding this comment.
tx->getResult().result.code(txINSUFFICIENT_FEE);
There was a problem hiding this comment.
What was decided on the tx endpoint: when we return txINSUFFICIENT_FEE, should we return some other information like fee required? (can be in XDR if this is only for Bump Fee or on the side)
There was a problem hiding this comment.
I don't think we ever came to a final conclusion on this. I'd rather not do the change in XDR. I think adding a field to the JSON response like "recommended_fee" would probably be best. This assumes that we want a "submit-then-get-recommendation" model, but other models like "get-recommendation-before-submit" are also possible. We should follow up with the Horizon team to see if they have any input on how this interface should work (@ire-and-curses).
jonjove
commented
Feb 28, 2020
|
List of warnings I am getting off 02a1fd614ed6a944a3df1734ae50458d2d11cb79 |
|
Patch for VS project |
MonsieurNicolas
requested changes
Mar 6, 2020
src/herder/TransactionQueue.cpp
Outdated
| { | ||
| int64_t oldFee = (*oldTxIter)->getFeeBid(); | ||
| if (fee < FEE_MULTIPLIER * oldFee) | ||
| { |
There was a problem hiding this comment.
What was decided on the tx endpoint: when we return txINSUFFICIENT_FEE, should we return some other information like fee required? (can be in XDR if this is only for Bump Fee or on the side)
jonjove
force-pushed
the
fee-bump-transactions
branch
from
02a1fd6
to
36c59b5
Compare
|
Latest warnings: |
MonsieurNicolas
requested changes
Mar 18, 2020
|
Left a few comments, mostly cosmetic @jonjove |
This was referenced Mar 18, 2020
sisuresh
reviewed
Mar 18, 2020
MonsieurNicolas
requested changes
Mar 18, 2020
tamirms
reviewed
Mar 20, 2020
| // because it is be used to represent the result of a Transaction. | ||
| struct InnerTransactionResult | ||
| { | ||
| int64 feeCharged; |
There was a problem hiding this comment.
is there ever a case where feeCharged will not be zero? It seems that the test cases in src/transactions/test/FeeBumpTransactionTests.cpp assert that the feeCharged in the inner transaction result is zero. If that's the case then it would be helpful to add a comment indicating that feeCharged should be ignored and it's only there so that InnerTransactionResult is binary compatible with TransactionResult.
MonsieurNicolas
requested changes
Mar 22, 2020
jonjove
force-pushed
the
fee-bump-transactions
branch
from
ca02835
to
8780721
Compare
|
r+ 8780721 |
This was referenced Apr 24, 2020
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.
Implements https://github.com/stellar/stellar-protocol/blob/master/core/cap-0015.md
Checklist
clang-formatv5.0.0 (viamake formator the Visual Studio extension)