Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rmrk-market: should unlisting stale listings be (1) open and (2) incentived? #82

Open
bmacer opened this issue Mar 1, 2022 · 3 comments

Comments

@bmacer
Copy link
Contributor

bmacer commented Mar 1, 2022

in the case where a listing is expired, anyone should be able to unlist it. we want to maintain clean storage in general, correct?
furthermore, should we even incentivize cleaning up this stale storage like with a deposit for a listing that gets refunded on purchase?

@HashWarlock
Copy link
Contributor

That could be beneficial to reduce storage. I was thinking about how to utilize #[pallet::hooks] and clean stale listings every certain number of Blocks. I did something similar with being able to auto increment the Era every certain number of blocks, but we can define a helper function and execute that or directly removes stale storage.

Example of auto increment Era:
https://github.com/HashWarlock/rmrk-substrate/blob/phala-world/pallets/phala-world/src/lib.rs#L214-L230

Tests I wrote for it:
https://github.com/HashWarlock/rmrk-substrate/blob/phala-world/pallets/phala-world/src/tests.rs#L78-L100

@bmacer
Copy link
Contributor Author

bmacer commented May 3, 2022

One challenge here is that the ListedNfts struct contains this expires info, so to access this, we'd need to iterate, which wouldn't be tenable. Alternatively, we could maintain a different struct, such as Expirations that storage expiration info. The downside of this is having to maintain this storage for buy, list and unlist.

Proposed storage would be...

	#[pallet::storage]
	#[pallet::getter(fn expirations)]
	/// Stores Expiration mapping Block Expiring -> 
	pub type Expirations<T: Config> = StorageMap<
		_,
		Blake2_128Concat,
		T::BlockNumber,
		BoundedVec<(CollectionId, NftId), T::MaxExpirationsPerBlock>,
		ValueQuery
	>;

Thoughts on this?

@HashWarlock
Copy link
Contributor

I believe we want to reduce the amount of reads and writes to storage and managing this for 3 separate storage could get complicated to manage. This would also create an increase in storage size. Could we potentially handle this expiration list off-chain in the frontend then have a marketplace admin account periodically call a privileged purging function to remove expired listings from ListedNfts? We would need to create an RPC API function to retrieve the listings that contain an expiration and then we can manage the expired NFTs through a script without needing to implement the logic on the blockchain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants