featureDisallowIncoming: Opt-out of incoming Checks, PayChans, NFTokenOffers and Trustlines
#4336
Conversation
RichardAH
force-pushed
the
incoming_nftoffer
branch
from
c2ce4c7
to
2003dec
Compare
RichardAH
changed the title
featureDisallowIncoming: Opt-out of incoming Checks, PayChans and NFTokenOffers
…sts for new flags
|
|
Yes. We need this. Users need this. The current situation confuses users. Thanks for the quick Dev 😬 |
|
@RichardAH Just a head's up that Scott S. is away this week, I don't think he'll be able to review this until next week. |
|
What's the envisioned user experience in cases where users change their mind? e.g. they don't think they want NFT offers, but later try to use the feature. What prevents confusion about the feature being blocked? And then when they do - intentionally - want to allow incoming NFT offers, what prevents users from being spammed anyway? I know that the PR description states that ignoring such objects on the client side is an inferior solution, but I think we should dig into this a bit further, and discuss it for the benefit of developers who read about this amendment in the future: Wallets could ignore such objects by default (and not show them to users). Similar to spam email, the wallet could run some kind of simple algorithm to determine whether the user should be shown the unsolicited object. Introducing these three account flags won't eliminate spam on the ledger, as it's a public blockchain and there are myriad different ways to spam (for example - even to the extent of creating new accounts, which may or may not be controllable by the spammer). |
There was a problem hiding this comment.
I like this functionality. I think we need to consider (separately) a flag that marks an account as “not incoming trustlines” which prevents the creation of trustlines to the account.
I only gave this a quick look, but it looks fine. Mark this as approved by me.
|
Thanks @nbougalis And @intelliot: I do not believe client side filtering is the right solution. We're living in 2022 and still receiving spam in our mailbox, and still instructing people to check their spam box. On top of that, all resources to send, transfer, receive, filter email are still being wasted. We have a shot here at preventing that hot mess on the XRPL, by simply allowing users to opt out of certain objects. (Example with NFTs, same applies for other objects) If I own no NFTs, why would I possibly want to receive offers to buy one? Even if my client filters it: it pollutes the ledger, it pollutes my account history, and it moves the responsibility to keep things clean to hopefully one day 100 different clients, instead of the single source of truth where something unwanted could be stopped at the source. And with the above example in mind: if I, one day, want to purchase an NFT, I use a random marketplace: the market place asks me to sign in, sees my r-address, sees I have the flag set not to receive offers, and tells me to change that flag in one simple swipe. Streamlined UI, lot of support tickets, forever persisting history pollution and wasted ledger resources prevented. |
The flags are easy for a platform to check. When you sign into that platform they check to see if your account is in the correct state to receive the objects their platform uses, and if it isn’t then give you a sign request to change your account settings.
Creating new accounts still won’t allow you to send NFTOffers to an account who has asfDisallowIncomingNFTOffer. As the ledger gets more complicated it is (highly) likely that different users will have different use cases. A bank doesn’t necessarily want to receive NFTOffers and an NFT trader doesn’t necessarily want to receive payment channels. At best receiving (being the target of) objects you don’t want or understand is annoying at worst it is a compliance (“how do we avoid putting this on our books”) and fear (“am I hacked?”) issue for end users. The XRPL already started down the path of ledger-side blocking with asfRequireAuth and asfRequireDest. It is very inconsistent to then add random new features without a way to also restrict these affecting an account and say “let the wallet deal with it”. If we are going to do that then I propose removing the existing flags too |
I was going to add such a flag but then realised asfRequireAuth should already do this? Incidentally blocking trustline creation is a form of freezing. It needs to be done carefully as not to contradict other behaviour |
It doesn't: even if you have |
There was a problem hiding this comment.
I'm in favor of this. I left a few nits, but I'm giving a thumbs-up now (however you want to resolve those is fine with me, including keeping it as-is).
My only concern is we are using one flag per object type, and we have a limited number of flags. Every time we add a new ledger object that requires permission we need to use another flag. Having said that, it's not a huge concern and this is good as-is. Thanks Richard! 👍 from me.
Ah gotcha. Happy to add another flag for 'non-issuing' in that case. I think we avoid any risk of it being used as another type of freeze by only allowing the flag to be set if the account has no issued balances on any of their trustlines. Can't freeze what doesn't exist.
Thanks for the review, I'll update the PR tomorrow with your recommendations in mind and the above additional flag. If flag exhaustion is a serious issue then we could move the Another slightly more hacky solution is to wait until we have actually exhausted flags and then just add an |
|
After some discussion here in the office with @WietseWind we decided that having existing outstanding assets (issued IOUs) should not block On the contrary we decided using this adds a new use-case to the XRPL: Maintaining a currency only tradable between trusted parties. First you setup all the trustlines you will ever allow then you enable the flag. Now you have an isolated currency. (Credit: @WietseWind) Trustlines can still be created in two additional ways, neither of which are checked by the new flag: offer crossing and checkmakestrustline. We figure no check is necessary here because it is something the user is initiating that produces the result. If you don't want the result then don't initiate the action. |
RichardAH
changed the title
featureDisallowIncoming: Opt-out of incoming Checks, PayChans and NFTokenOffersfeatureDisallowIncoming: Opt-out of incoming Checks, PayChans, NFTokenOffers and Trustlines
|
Hi @scottschurr thanks for the code review |
|
@WietseWind could we get a quick confirmation that this still resolves the issue for users? Thank you! |
|
Just mentioning @mDuo13 for awareness since this change will cause the need for a documentation update on XRPL.org |
Sure does! |
…kenOffers and Trustlines (XRPLF#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline
…kenOffers and Trustlines (XRPLF#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline
…kenOffers and Trustlines (XRPLF#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline
* Follow-up to #4336 * NFToken is the naming convention in the codebase (rather than NFT) * Rename `lsfDisallowIncomingNFTOffer` to `lsfDisallowIncomingNFTokenOffer` * Rename `asfDisallowIncomingNFTOffer` to `asfDisallowIncomingNFTokenOffer`
…kenOffers and Trustlines (XRPLF#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline
* Follow-up to XRPLF#4336 * NFToken is the naming convention in the codebase (rather than NFT) * Rename `lsfDisallowIncomingNFTOffer` to `lsfDisallowIncomingNFTokenOffer` * Rename `asfDisallowIncomingNFTOffer` to `asfDisallowIncomingNFTokenOffer`
|
Hello, |
|
@ckeshava See: https://xrpl.org/amendments.html#:~:text=Amendment%20Voting,-Each%20version%20of&text=Operators%20of%20rippled%20validators%20configure,can%20change%20between%20software%20releases. It has to exceed 80% for at least two weeks, without dropping below >80% in those two weeks. |
|
@WietseWind thanks, yes I understand that. But since this PR was merged about 6 months ago, I am curious to know if there is any specific reason why it is not voted in by the majority of the community. I guess it's impossible to know the intentions/reasons of all the validators. cc @MorganBergen : This PR is related to one of your projects. |
|
@ckeshava this PR was merged 6 months ago but included in the 1.10 release in March 2023 https://xrpl.org/blog/2023/rippled-1.10.0.html. The voting time for this particular amendment seems reasonable right now Voting by validators naturally takes time to evaluate and we also want nodes to all upgrade to the version to avoid amendment blocking |
…kenOffers and Trustlines (XRPLF#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline
High Level Overview of Change
Allow users of the XRPL to signal (and enforce) that they do not wish to accept an incoming instrument of any (or all) of the following:
Context of Change
Significant support resources at XUMM are being wasted by end users who are on the unsolicited receiving end of these objects. While a client side block is possible, it is better for the ledger that unsolicited objects are never created in the first place.
If enabled the following three AccountSet Flags are added:
Their behaviour and usage is exactly as you'd expect so I won't detail it.
Type of Change
Test Plan
Tests have been added in PayChan_test.cpp, Check_test.cpp, NFToken_test.cpp and SetTrust_test.cpp