Feedback on Fairminters

[droplister]

Looking forward to mints, did not have time to review the spec until now. Here is some feedback:

commision:
I don't think it makes sense to set a max commission, but UI/UX developers should alert people if commission is 100%.

asset issued if soft cap unmet:
Does the asset get issued if soft cap is unmet? This makes sense, but it may be good to mention this somewhere.

max start and end block:
One of the issues I see with dispensers is they never expire which over time encumbers the network with maintaining their functionality indefinitely, as-is. We could lessen this impact for fairminters with a max start and end block, relative to the current block. (5-10 years?)

max soft and hard cap check:
Could make sense to check against config.MAX_INT like how issuance does.

asset_parent and {asset_parent}.{asset} concatenation:
This is unlike other parts of the codebase. I think people may accidentally issue "PARENT.PARENT.ASSET" with this setup.

counterparty-core/counterparty-core/counterpartycore/lib/messages/fairminter.py

Lines 133 to 134 in 827ae09

	if asset_parent != "":
	asset_name = f"{asset_parent}.{asset}"

There is a parse_subasset_from_asset_name utility that could be used and then the compose fairminter call could accept just an asset like how an issuance works. I also think using subasset_parent instead of asset_parent would be more consistent. There may be other utils for subasset names that could be shared.

counterparty-core/counterparty-core/counterpartycore/lib/messages/issuance.py

Lines 426 to 428 in 827ae09

	subasset_parent, subasset_longname = util.parse_subasset_from_asset_name(
	asset, util.enabled("allow_subassets_on_numerics")
	)

[ouziel-slama]

Thank you @droplister for this feedback and sorry for the late answer!

commision: I don't think it makes sense to set a max commission, but UI/UX developers should alert people if commission is 100%.

+1

asset issued if soft cap unmet: Does the asset get issued if soft cap is unmet? This makes sense, but it may be good to mention this somewhere.

If sof cap is unmet, the assets are destroyed.

max start and end block: One of the issues I see with dispensers is they never expire which over time encumbers the network with maintaining their functionality indefinitely, as-is. We could lessen this impact for fairminters with a max start and end block, relative to the current block. (5-10 years?)

hum.. why prevent a fairminter from remaining open forever?

max soft and hard cap check: Could make sense to check against config.MAX_INT like how issuance does.

this is done here https://github.com/CounterpartyXCP/counterparty-core/blob/master/counterparty-core/counterpartycore/lib/messages/fairminter.py#L40-L57

asset_parent and {asset_parent}.{asset} concatenation: This is unlike other parts of the codebase. I think people may accidentally issue "PARENT.PARENT.ASSET" with this setup.

is indeed possible to create a fairminter for a sub-sub-asset but not with a "." in the name.

counterparty-core/counterparty-core/counterpartycore/lib/messages/fairminter.py

Lines 133 to 134 in 827ae09

if asset_parent != "":
asset_name = f"{asset_parent}.{asset}"
There is a parse_subasset_from_asset_name utility that could be used and then the compose fairminter call could accept just an asset like how an issuance works. I also think using subasset_parent instead of asset_parent would be more consistent. There may be other utils for subasset names that could be shared.

counterparty-core/counterparty-core/counterpartycore/lib/messages/issuance.py

Lines 426 to 428 in 827ae09

subasset_parent, subasset_longname = util.parse_subasset_from_asset_name(
asset, util.enabled("allow_subassets_on_numerics")
)

yes definitely this part of the code could be improved but of course you have to be super careful here not to cause a consensus problem

[adamkrellenstein]

See #3006