-
Notifications
You must be signed in to change notification settings - Fork 570
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
Incorrect implementation of the disable-bit smartcoin issuer permissions & flags? #3732
Comments
Working on the astro-ui, having all issuer permission toggles set to true (enable-bits all enabled, disable-bits all actively disabling) then I too get 65535, only when I set all enable-bits to true, and all disable-bits to false toggles do I get 511 for issuer_permissions.
Do you mean that the disable-bits should only contribute their following values to the issuer_permission/flag calculations when their toggles have been set to false?
Code for how I calculate issuer permissions: https://github.com/BTS-CM/astro-ui/blob/main/src/lib/common.js#L140 |
Another related query - should we be offering a global settlement flag to configure?
The docs state this is a permission only, so the flag should be permanently set to false, right? The asset issuer can perform the asset_global_settle with the permission set to true & its associated flag set to false? Currently the reference UI offers the global_settle flag for possible activation, if its associated enable-bit permission is true, which I believe will cause the asset_create operation to be rejected by the network, right? |
For permission only bits, the flag can only be set to 0.
|
Generally speaking, setting the issuer permissions to 511 means that the issuer is allowed to do everything, which is the default value set by older versions of bitshares-ui. New versions should retain this behavior and be compatible. By "allowed" I mean that the issuer has not waived this right. |
Note: Editing in response to feedback
This is how I am currently understanding it, the reason why 511 isn't what's being calculated is because each of the new disable-bit issuer permissions are providing their hex value towards the final value when default checked true (which is by default disabling the feature) Should it be the opposite, ie disable_mcr_update true = 0 and false = 0x800 then? |
Not exactly.
Correct. Others should be inline with this narrative.
Not exactly. (Also the same for the following 3.)
Correct.
By default all permissions should be enable. That being said, the enable-bits should all be 1, and the disable-bits should all be 0. (I don't use "true" or "false" to describe it.) |
Why "Unable to set static MCR" is not correct? Because the underlying logic is "Unable to UPDATE MCR if static MCR is already set; and unable to set static MCR if static MCR is not already set".
|
I've continued to edit my posts above with my latest understanding of the permissions/flags/locales
I used true to represent a green checked active checkbox in the UI & false to represent the grey unchecked checkbox, in Javascript both With regards to what you mean above that disable-bits should all be 0, the current UI implementation of a disable-bit is as follows:
Do you mean it should be as follows instead for each of the disable-bit issuer permissions?
However, for a disable-bit issuer permission's relevant flag it should remain the other way around, like so since it's an enable-bit flag instead of a disable-bit flag?
A follow up question also is that when you disable an enable-bit issuer permission it's a permanent move if the supply is greater than 0, is the reverse true for disable-bit issuer permissions that the move from 0 to 1 is a permanent issuer permission configuration? |
By 1 or 0 I actually mean the "Resulting Hexadecimal Literal Value". |
As I've said these kind of description was correct.
I say these were "not exactly" which means they don't include all the underlying logic. In other words, these were wrong. |
Yes. Bits in flags are all enable-bits. However please note that some bits are to disable something..
Yes. |
I've updated the asset effects tables for reference So, when you want all the disable-bits enabled by default, you want 511 issuer permission, but when you want all enable-bits + all disable-bits disabled, what issuer permission value are you expecting? 65535 An exhaustive list of every permission value + effect + hex value for issuer permissions and flags would be greatly appreciated @sschiessl-bcp @gibbsfromncis @HarukaMa @svk31 @startailcoon Any thoughts on how we should implement the required changes in the reference UI? |
To be clear, I want all FEATURES to be "can be enabled" by default, not every bit set to 1 by default. It means I want enable-bits be 1 and disable-bits be 0 by default, thus 511. |
The permissions and flags are both 16 bit in data storage (please think in binary form). Each bit represents a feature. The right-most bit represents "enable market fee", the left-most bit represents "disable collateral bidding", etc. When I say a bit to be 0 or 1, I literally mean that one bit in binary form to store 0 or 1. I want the default permissions to be I want the default flags to be 0. It means all the features are disabled. For UI/UX, I want to avoid "double negatives" for simplicity and clarity. Because typical users don't care how the data is stored, they only care about functionalities. The code should correctly translate what the user decides on the UI into data, and that's what programmers do. I will add a data table later. |
I've changed the Astro UX to use the same permission/flag wording as the reference UI, minus expanding the acronyms to full wording to avoid translation into incorrect acronym definitions, so the user isn't affected by the double negative terminology. Now that I've adjusted the issuer permission to the new spec, the above yields:
|
Asset Issuer Permissions
Asset Flags
|
I don't think wording in the reference UI is perfect, but it is a good start. Thanks. We know the system is complicated and hard to make perfect UI/UX. |
Updated my comment #3732 (comment) with a full list of asset issuer permissions and flags. |
Update: fixed some errors in my comment #3732 (comment), and added more info to some items. |
Describe the bug
The new disable-bit permissions are as follows:
So, if my understanding of the above is correct, then the following should be true:
This applies for the rest of these disable-bit flags, so when the toggle is disabled, the flag should become available to edit, right?
When you disable the issuer permission, the corresponding flag is missing, you're therefore unable to enable the flag to enable the new feature?
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Unlike the enable-bit issuer permissions, the disable-bit issuer permissions should result in the opposite behaviour in the flags - they should be visible and editable flags when their disable-bit issuer permission has been disabled.
Additional context
Core code reference: https://github.com/bitshares/bitshares-core/blob/3174d22b5267d0057d0857ac53086e9813c8559f/libraries/protocol/include/graphene/protocol/types.hpp#L205
This is the point where the disabled disable-bit permission fails to be included as a configurable flag:
bitshares-ui/app/components/Account/AccountAssetCreate.jsx
Line 771 in cba8821
The double-negative terminology is confusing.
Misconfiguration of these disable-bit's corresponding flags could be preventing users from creating new smartcoins.
The text was updated successfully, but these errors were encountered: