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

Polish BSIP40 with latest change requests #2203

Open
wants to merge 10 commits into
base: release
Choose a base branch
from

Conversation

nathanielhourt
Copy link
Contributor

This addresses abit's change requests wrt BSIP40 in #2093

See https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r401052309

A compile error is now thrown if any operation field type is unsupported
in use in any predicate with any argument type.

See https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r400286103

Unsigned_int is now supported as an integral type
See bitshares#2093 (comment)

Add a cmake template file for the list_#.cpp files and autogenerate them
from the template rather than having 12 almost identical copies of a file
See: https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r401604111

I can still get away with reordering the list like this because it's
never been released yet.
Now allows ==, !=, <, <=, >, >= for stringish types (including hashes
and vector<char>s), container size vs int, and static_variant.which()
values vs int.

vector<char> is no longer considered a container, but is considered
stringish

See:
https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r401604111
https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r401635775
https://github.com/bitshares/bitshares-core/pull/
2093#discussion_r401642590
Add support for innteger comparisons against a stringish that is not a
container, necessary because vector<char> is no longer a container but
only stringish.
/* 7 */ fc::sha256, \
/* 8 */ fc::ripemd160, \
/* 9 */ fc::hash160, \
/* 10 */ account_id_type, \
Copy link
Member

@abitmore abitmore Jun 30, 2021

Choose a reason for hiding this comment

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

Unfortunately the reordering breaks the public testnet.

[54, {"fee"=>{"amount"=>300000, "asset_id"=>"1.3.0"}, "account"=>"1.2.24713", "enabled"=>true, "valid_from"=>"2019-11-15T00:00:00", "valid_to"=>"2019-11-30T00:00:00", "operation_type"=>0, "auth"=>{"weight_threshold"=>1, "account_auths"=>[["1.2.24713", 1]], "key_auths"=>[], "address_auths"=>[]}, "restrictions"=>[{"member_index"=>2, "restriction_type"=>0, "argument"=>[7, "1.2.23833"], "extensions"=>[]}], "extensions"=>[]}]
[55, {"fee"=>{"amount"=>230000, "asset_id"=>"1.3.0"}, "account"=>"1.2.24713", "authority_to_update"=>"1.17.0", "new_auth"=>{"weight_threshold"=>1, "account_auths"=>[["1.2.24714", 1]], "key_auths"=>[], "address_auths"=>[]}, "restrictions_to_remove"=>[], "restrictions_to_add"=>[], "extensions"=>[]}]
[54, {"fee"=>{"amount"=>330000, "asset_id"=>"1.3.0"}, "account"=>"1.2.24713", "enabled"=>true, "valid_from"=>"2019-11-15T00:00:00", "valid_to"=>"2019-11-30T00:00:00", "operation_type"=>1, "auth"=>{"weight_threshold"=>1, "account_auths"=>[["1.2.24713", 1]], "key_auths"=>[], "address_auths"=>[]}, "restrictions"=>[{"member_index"=>2, "restriction_type"=>10, "argument"=>[39, [{"member_index"=>1, "restriction_type"=>0, "argument"=>[8, "1.3.0"], "extensions"=>[]}]], "extensions"=>[]}], "extensions"=>[]}]
[55, {"fee"=>{"amount"=>230000, "asset_id"=>"1.3.0"}, "account"=>"1.2.24713", "authority_to_update"=>"1.17.1", "new_auth"=>{"weight_threshold"=>1, "account_auths"=>[["1.2.24714", 1]], "key_auths"=>[], "address_auths"=>[]}, "restrictions_to_remove"=>[], "restrictions_to_add"=>[], "extensions"=>[]}]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh? I didn't realize this had been released, much less used. The fix is just to move the new hash types to the bottom of the list, no?

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.

2 participants