Proposed 1.12.0-rc4 #4685
Merged
Proposed 1.12.0-rc4 #4685
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
Modify two error cases in AMMBid transactor to return `tecINTERNAL` to more clearly indicate that these errors should not be possible unless operating in unforeseen circumstances. It likely indicates a bug. The log level has been updated to `fatal()` since it indicates a (potentially network-wide) unexpected condition when either of these errors occurs. Details: The two specific transaction error cases changed are: - `tecAMM_BALANCE` - In this case, this error (total LP Tokens outstanding is lower than the amount to be burned for the bid) is a subset of the case where the user doesn't have enough LP Tokens to pay for the bid. When this case is reached, the bidder's LP Tokens balance has already been checked first. The user's LP Tokens should always be a subset of total LP Tokens issued, so this should be impossible. - `tecINSUFFICIENT_PAYMENT` - In this case, the amount to be refunded as a result of the bid is greater than the price paid for the auction slot. This should never occur unless something is wrong with the math for calculating the refund amount. Both error cases in question are "defense in depth" measures meant to protect against making things worse if the code has already reached a state that is supposed to be impossible, likely due to a bug elsewhere. Such "shouldn't ever occur" checks should use an error code that categorically indicates a larger problem. This is similar to how `tecINVARIANT_FAILED` is a warning sign that something went wrong and likely could've been worse, but since there isn't an Invariant Check applying here, `tecINTERNAL` is the appropriate error code. This is "debatably" a transaction processing change since it could hypothetically change how transactions are processed if there's a bug we don't know about.
- Update amm_info to fetch AMM by amm account id. - This is an additional way to retrieve an AMM object. - Alternatively, AMM can still be fetched by the asset pair as well. - Add owner directory entry for AMM object. Context: - Add back the AMM object directory entry, which was deleted by #4626. - This fixes `account_objects` for `amm` type.
intelliot
requested review from
injaelee,
legleux,
gregtatcam,
manojsdoshi,
sgramkumar and
scottschurr
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.
High Level Overview of Change
Compared to rc3, this version includes the following:
Context of Change
In the process of documenting AMM, two "impossible" error cases were identified. These now return
tecINTERNALbecause they should never occur; if they occur, please open an issue to ensure the mechanism of action is investigated and understood.With the experience of using the AMM APIs, it was determined that it may be useful to retrieve AMM with
account_objects, so the AMM object owner directory entry was added back in. Additionally,amm_infowas enhanced to also allow users to retrieve an AMM object by amm account id.Type of Change