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

BSIP 75 - Asset Owner defines MCR and MSSR values #218

Merged
merged 11 commits into from
May 22, 2020
Merged

Conversation

jmjatlanta
Copy link
Contributor

See Issue #96

  • Add specifications
  • Verify rationale
  • Research risks

@jmjatlanta jmjatlanta marked this pull request as ready for review October 1, 2019 12:18
bsip-0075.md Outdated Show resolved Hide resolved
bsip-0075.md Outdated Show resolved Hide resolved
bsip-0075.md Outdated

The `update_asset_operation` shall be modified permit updating of the MCR and MSSR values, if the asset has been created with the permission for the asset owner to do so. NOTE: much of the logic currently in `asset_publish_feeds_evaluator::do_apply()` (i.e. call order checks and calculations) shall be also called here if MCR or MSSR are adjusted.
The `update_asset_operation` shall be modified to permit updating of the MCR and MSSR values, if the asset has the appropriate `flags` value set permitting the asset owner to do so. NOTE: much of the logic currently in `asset_publish_feeds_evaluator::do_apply()` (i.e. call order checks and calculations) shall be also called here if MCR or MSSR are adjusted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the prior text, which references the pre-existing permission setting of the asset, is more correct

bsip-0075.md Show resolved Hide resolved
bsip-0075.md Outdated
# Rationale
Prior to this change, adjustments to MCR and MSSR were handled by feed producers. This often requires the asset owner to contact the feed producer and ask for the change. For some assets, such a step is time consuming for both parties and ill-fitted as part of the feed producer's responsibilities.

This new change shall permit the asset owner to determine *at asset creation time* who should have the responsibility to maintain those values.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will existing smartcoins be affected?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing smartcoins aren't affected. The owners can change settings after the protocol upgrade though.

@abitmore
Copy link
Member

I wonder why this hasn't been merged.

@abitmore
Copy link
Member

I got a better idea about specification and implementation lately: just add new optional value fields in bitasset options, which can be updated by the asset owners:

struct bitasset_options 
{
   //...
   optional<uint16_t> mcr;
   optional<uint16_t> mssr;
   optional<uint16_t> icr;
};

If one of the parameter is set, it means the owner has an opinion on the parameter and it will be in effect; if not set, then the value provided by the feed producers will be in effect.

We can assign a permission bit on each parameter too, so the owners can decide whether to forfeit the right to update it.

@MichelSantos
Copy link
Contributor

We can assign a permission bit on each parameter too, so the owners can decide whether to forfeit the right to update it.

I proposed a commit to include permissions. I left out references to ICR because I thought that it might be out of scope. If appropriate I have text which to update BSIP77 with a similar discussion about permissions.

@abitmore
Copy link
Member

Yes I think BSIP77 can be updated similarly. Thanks.

@abitmore abitmore merged commit 6d7a41d into master May 22, 2020
@abitmore abitmore deleted the jmj_bsip_75 branch May 22, 2020 09:30
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

Successfully merging this pull request may close these issues.

4 participants