Hi @Phoenix616 This is the second version of this request. Still working out how best to describe the problem, and what the most elegant option is which can handle this problem.

Is your feature request related to a problem? Please describe.

Players can create shops that empty into a hopper. This adds all kinds of useful shop configurations. As long as the player has the shop claimed and has build permissions, then there's no problem. If the player loses build permissions on some of these useful shop configurations then the new land owner can change the configuration to empty the shop and continue selling to it, basically forever. This is pretty bad for the former shop owner, and it's an easy mistake to make.

Describe the solution you'd like

Config option to restrict ChestShops to land claims. AND
Config option to close ChestShop if the land is unclaimed OR if the shop owner does not have build permissions in the claim. (in GP three permission levels have build: trust/management/ownership)

It seems like figuring out when to check and close the shop is an interesting challenge. One option that you've kind of suggested is to wait until a player transacts with the shop to check if the shop owner has build perms.

Describe alternatives you've considered

ChestShop offers to block hoppers from drawing from chests. This solves the problem but at the cost of some functionality.

We've discussed some alternatives in this thread. Like closing shops if the player is inactive x days. This option is a bit imprecise because the shop may still be protected by land claims where they can build. And for extra points: at the moment I have shop owners who are also subscribed donors. Subscribers have persistent land claims. They wont be too happy if their protected shops end up closed.

Additional context

In LMC lots of players have created 'super shops' where several ChestShops are connected via hoppers to offer a variety of prices. Supply & Demand stuff. Other shops feed directly into automated farms and back into shops. All these configurations are brilliant. It's one of the coolest things the players are doing with ChestShop right now.

If those shops become unprotected or if the Shop Owner loses build perms, then a malicious attacker will be able to access their entire player balance by selling to the shop, emptying it out, repeat.

If #277 adds a "restrict chestshop trading to Land Claim where ShopOwner has build permissions." then that single feature would solve this issue.

GP has a check for build permissions. Idk what it is, but GP uses it when players run the /trapped command.

Currently all the protection plugin integrations simply check if there is a region and only allow shop creation inside of regions, not check the build permissions for the actual transaction. I guess such a feature could be added but practically it might not be easily possible as most protection plugin APIs (GriefPrevention included) seem to require an object of an actual online player so that check would need to be done some other way (e.g. directly with the storage or manually comparing owner names) and that could get a bit messy.

I feel like the original general inactivity check idea might be of more use and easier to implement than some (kinda arcane) region owner check for transaction, especially as it wouldn't necessarily need to add support for different protection plugins.

OK, it sounds like the existing protection plugins are a good starting point.

Maybe this isn't something you want to do, but just in case you don't want to lose the chance at some paid development work I'll try and sketch it out. Using GP as an example. Let's imagine shops can only be created in land claims. Simple start, because it sounds like the GP protection plugin integrations check for a Land Claim. Here's the twist. If the claim is abandoned, OR if the shop owner loses build permissions in the claim, then update the shop sign:

L0: [Player Name]
[L2]: Closed Shop

And I'm open to alternative ideas for updating the shop to close it.

Build permissions are available in three ways. Ownership, Management, Trust.

Well rewriting the signs wouldn't really be ideal as that would require checking all signs in the region to see if they are a shop when it is detected that the claim changed (which in itself might be an issue if no event is emitted there by the protection plugin)

A simple check when a player tries to trade at the shop and then send an appropriate message would be easier and cleaner to implement. (Optionally the shop sign could update I guess but that could be inconvenient as then information about the shop goes lost that the owner couldn't easily restore if he comes back but I guess the owner line could be updated with something like "Closed Shop")

OK, it looks like there's going to be some ideas which in practice don't turn out to be that simple. Or perhaps they end up using more resources than another solution can manage. Elegant solutions are generally my preference.

Sometimes things which are really important to me might not be obvious. So it's possible I'll have to go back and make a change, and it might seem trivial or unnecessarily complicated. I highly value a developer who can work with that. For context, heres some work RoboMWM and I did GPAuctions' closed issue log.

Preserving information that players may value (historical prices/rates) seems interesting to me.

Do you think the owner line "Closed Shop" might conflict with a player named "Closed Shop"?

And before we continue, I'm taking your interest here to mean that you might want to take on this paid development work. What else do you want to know before making a commitment?

Well "Closed Shop" shouldn't be incompatible with existing shops seeing as player names can't include spaces but this could just be exposed in the config the same way as the admin shop name is if server owners notice that it's an issue.

As for paid work: I'm always open to suggestions that might improve this plugin and for other users to be able to benefit from it. Of course I personally will often prioritise features that might be beneficial to my own servers or just be more interesting over other ones so if you want to provide some monetary incentive for certain features it probably will get them added faster. Maybe this could even be done via a bounty program so that other people might have the ability to implement it too (unfortunately it looks like bountysource which was used previously has issues right now :S Edit: Added issuehunt support. Of course you don't have to use that, just as a general note for anyone stumbling over this when searching for bountsource) or if you want me personally to do some stuff feel free to send me a message on spigot or an email.

For now I'm going to shelve this request.

I want to do it, but maybe there's better uses of funding. Even including other ChestShop updates that can arise.

Last week I saw an interview with Andrew Ng, creator of Coursera, world renowned cryptography and AI researcher. Well, they invested a lot of $ and time building a way for several users to watch the same video on the same computer, and everyone gets credit for watching the lecture. And nobody uses it. Nobody.

It seems like this vulnerability, which has never been an issue for anyone, might be one of those cases. Still, I really appreciate the feedback and look forward to paying you money for something else.

You're welcome to ignore this post for a while. I'm going to boil it down to way fewer words so it's clearer, wastes less of your time.

What exactly do you mean by "draining"? That people can sell to the shop a lot and therefore get all of their money eventually? If so it's generally suggested to block the slots of the container so that not too much can be sold. (As suggested in this guide on the project page, but I guess that doesn't work in your case with hoppers)

It should be pretty simple to just cancel the transaction event in another extension plugin when the player is offline or wasn't seen for a long time. Not sure if it's really necessary to add to ChestShop directly (unless others are interested in this as well I guess)