Conversation
| if (ctx_.view().rules().enabled(featureIOUIssuerWeakTSH)) | ||
| { | ||
| additionalWeakTSH_.emplace(lowAcc); | ||
| additionalWeakTSH_.emplace(highAcc); |
There was a problem hiding this comment.
The code in which additionalWeakTSH_ is processed probably needs to be changed to ensure if an account appears twice as a result of this change that it will always use the strongest TSH status
xahaud/src/ripple/app/tx/impl/Transactor.cpp
Line 1514 in 29b978b
There was a problem hiding this comment.
strongTSH is added by getTransactionalStakeHolders(), and weakTSHs are added to the end of that vector list.
Therefore, in the alreadyProcessed processing, strongTSH won't be processed after weakTSH, right?
|
Need to resolve WeakTSH handling issue when transaction results in deletion of ledger object |
RichardAH
force-pushed
the
dev
branch
2 times, most recently
from
77b8c83 to
532a471
Compare
…ken(CreateOffer,AcceptOffer,CancelOffer
| // the burner is the owner and not the issuer of the token | ||
| // the burner is the issuer and not the owner of the token | ||
|
|
||
| if (iouIssuerWeakTSH && amount) |
There was a problem hiding this comment.
amount.has_value() is preferred way to test optionals
There was a problem hiding this comment.
I'm also wondering if this should be a weak tsh? This amount field on the uritoken is for a sale offer, but here the token is being destroyed. I guess it doesn't hurt but it's very very ancillary to the amount's issuer. None of their tokens change hands here.
There was a problem hiding this comment.
Yes, we need consistency.
- Only treat as TSH when there is a change in the IOU Balance
or
- Treat as TSH when IOU is specified in the transaction (regardless of whether there is a change in the Balance) (and Balance changed)
There was a problem hiding this comment.
Tbh it's kind of beyond a weak tsh, like "ultra weak" or ancillary or hypothetical tsh. Because the Issuer isn't a party to the transaction, isn't a stakeholder in the transaction, what we're actually saying is that they might become a party to a transaction involving this object later, and specifically in this case: that they will no longer be a party to a hypothetical future transaction involving this object later. Very ancillary indeed.
I guess my question is: does the meticulous chasing of these ancillary edges cases actually make weak tsh too expensive or less useful for its intended usecase? Should we instead add a third category of tsh for these cases and let them opt into that maybe? I suppose they can always use HookOn to ensure they're opting out, unless they're both an issuer for a currency and the issuer of uritokens.
| std::vector<hook::HookResult> weakResults; | ||
|
|
||
| doTSH(false, stateMap, weakResults, proMeta); | ||
| if (!view().rules().enabled(featureIOUIssuerWeakTSH)) |
There was a problem hiding this comment.
is this condition supposed to be inverted?
There was a problem hiding this comment.
This PR changes getTransactionalStakeHolders() to be called only once, but since a second call is still needed when the feature is disabled, it is called here.
I will add a comment.
…akeHolders` to generic processing using `addWeakTSHFromBalanceChanges`.
4 participants
Affected to
High Level Overview of Change
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)