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.

Sign up for GitHub

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

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Dev feature 3 + 6 - Mutable Templates & Non-Burnable or Non-transferable XOR condition #8

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6075419
dev-feature-3
on-a-t-break Sep 6, 2024
86b4708
dev-feeature-3-fixes
on-a-t-break Sep 15, 2024
4ff8b26
dev-feature-3-fixes
on-a-t-break Sep 15, 2024
35dbb88
dev-feature-3 + dev-feature-6 merge
on-a-t-break Sep 20, 2024
d4857de
polymorph / overloading fix
on-a-t-break Sep 20, 2024
4d69ac0
missing schema_name emplace on createtempl2
on-a-t-break Sep 20, 2024
e7a3241
Fixes; *performed all fixes; *still need to decide templatedata table…
on-a-t-break Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions include/atomicassets.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ CONTRACT atomicassets : public contract {
ATTRIBUTE_MAP immutable_data
);

ACTION createtempl2(
name authorized_creator,
name collection_name,
name schema_name,
bool transferable,
bool burnable,
uint32_t max_supply,
ATTRIBUTE_MAP immutable_data,
ATTRIBUTE_MAP mutable_data
);

ACTION locktemplate(
name authorized_editor,
name collection_name,
Expand All @@ -129,6 +140,13 @@ CONTRACT atomicassets : public contract {
ATTRIBUTE_MAP new_mutable_data
);

ACTION settempldata(
name authorized_editor,
name collection_name,
name schema_name,
Copy link

Choose a reason for hiding this comment

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

I don't think schema_name should be a part of the action signature, as it can be inferred from the template_id. setassetdata also doesn't include the schema name

int32_t template_id,
ATTRIBUTE_MAP new_mutable_data
);

ACTION announcedepo(
name owner,
Expand Down Expand Up @@ -233,6 +251,14 @@ CONTRACT atomicassets : public contract {
ATTRIBUTE_MAP new_data
);

ACTION logsetdatatl(
name collection_name,
name schema_name,
int32_t template_id,
ATTRIBUTE_MAP old_data,
ATTRIBUTE_MAP new_data
);

ACTION logbackasset(
name asset_owner,
uint64_t asset_id,
Expand Down Expand Up @@ -295,6 +321,16 @@ CONTRACT atomicassets : public contract {

typedef multi_index <name("templates"), templates_s> templates_t;

//Scope: collection_name
TABLE template_data_s {
int32_t template_id;
name schema_name;
vector <uint8_t> mutable_serialized_data;

uint64_t primary_key() const { return (uint64_t) template_id; }
};

typedef multi_index <name("templatedata"), template_data_s> template_data_t;
Copy link

Choose a reason for hiding this comment

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

I think calling this templatedata could be confusing, because template data already exists as part of the normal template table. A name that makes it clear this is referring to mutable template data would be better imo.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, wdy think about these:

  • templmutable
  • mutabletempl


//Scope: owner
TABLE assets_s {
Expand Down Expand Up @@ -370,6 +406,16 @@ CONTRACT atomicassets : public contract {
config_t config = config_t(get_self(), get_self().value);
tokenconfigs_t tokenconfigs = tokenconfigs_t(get_self(), get_self().value);

void create_template(
Copy link

Choose a reason for hiding this comment

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

I would call this internal_create_template to be consistent with the other internal functions.

name & authorized_creator,
Copy link

Choose a reason for hiding this comment

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

I am not a fan of passing names by reference, since they're essentially just 64 bit primitive types. Most likely compiles to exactly the same output anyways, so more of a nitpick than anything else.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, can be set to pass by value. I always do everything by reference when I can if I know that the original isn't going to be mutated.

name & collection_name,
name & schema_name,
bool transferable,
bool burnable,
uint32_t max_supply,
ATTRIBUTE_MAP & immutable_data,
ATTRIBUTE_MAP mutable_data = {}
Copy link

Choose a reason for hiding this comment

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

is there a reason not to use a reference for the mutable data when you're using one for the immutable data? I also don't really like the default value, I'd rather see that explicitly when calling the function.

Copy link
Author

@on-a-t-break on-a-t-break Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There's no reference for the mutable_data because the original createtempl action does not have a field for mutable_data (only immutable_data). It's only accessed by the new createtempl2 action.

Just looks a bit cleaner this way, as the function doesn't have to create an empty ATTRIBUTE_MAP & pass that over (if it was by reference)

);

void internal_transfer(
name from,
Expand Down Expand Up @@ -408,4 +454,6 @@ CONTRACT atomicassets : public contract {
schemas_t get_schemas(name collection_name);

templates_t get_templates(name collection_name);

template_data_t get_template_data(name collection_name);
};
248 changes: 203 additions & 45 deletions src/atomicassets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ ACTION atomicassets::extendschema(
* Creates a new template
* @required_auth authorized_creator, who is within the authorized_accounts list of the collection
*/

ACTION atomicassets::createtempl(
name authorized_creator,
name collection_name,
Expand All @@ -417,53 +418,26 @@ ACTION atomicassets::createtempl(
bool burnable,
uint32_t max_supply,
ATTRIBUTE_MAP immutable_data
) {
require_auth(authorized_creator);

auto collection_itr = collections.require_find(collection_name.value,
"No collection with this name exists");

check_has_collection_auth(
authorized_creator,
collection_name,
"The creator is not authorized within the collection"
);

schemas_t collection_schemas = get_schemas(collection_name);
auto schema_itr = collection_schemas.require_find(schema_name.value,
"No schema with this name exists");

config_s current_config = config.get();
int32_t template_id = current_config.template_counter++;
config.set(current_config, get_self());

templates_t collection_templates = get_templates(collection_name);
) {
create_template(authorized_creator, collection_name, schema_name, transferable, burnable, max_supply, immutable_data);
}

collection_templates.emplace(authorized_creator, [&](auto &_template) {
_template.template_id = template_id;
_template.schema_name = schema_name;
_template.transferable = transferable;
_template.burnable = burnable;
_template.max_supply = max_supply;
_template.issued_supply = 0;
_template.immutable_serialized_data = serialize(immutable_data, schema_itr->format);
});
/**
* Creates a new template with explicit mutable data fields
* @required_auth authorized_creator, who is within the authorized_accounts list of the collection
*/

action(
permission_level{get_self(), name("active")},
get_self(),
name("lognewtempl"),
make_tuple(
template_id,
authorized_creator,
collection_name,
schema_name,
transferable,
burnable,
max_supply,
immutable_data
)
).send();
ACTION atomicassets::createtempl2(
name authorized_creator,
name collection_name,
name schema_name,
bool transferable,
bool burnable,
uint32_t max_supply,
ATTRIBUTE_MAP immutable_data,
ATTRIBUTE_MAP mutable_data
) {
create_template(authorized_creator, collection_name, schema_name, transferable, burnable, max_supply, immutable_data, mutable_data);
}


Expand Down Expand Up @@ -666,6 +640,85 @@ ACTION atomicassets::setassetdata(
});
}

/**
* Updates the mutable data of a template within the templatedata table
* If the row doesn't exist within the template, it emplaces a new row
* If the new_mutable_data is empty & the row exists, it eraes the row
* @required_auth authorized_editor, who is within the authorized_accounts list of the collection
specified in the related template
*/


ACTION atomicassets::settempldata(
name authorized_editor,
name collection_name,
name schema_name,
int32_t template_id,
ATTRIBUTE_MAP new_mutable_data
) {

require_auth(authorized_editor);

auto collection_itr = collections.require_find(collection_name.value,
"No collection with this name exists");

check_has_collection_auth(
authorized_editor,
collection_name,
"The editor is not authorized within the collection"
);

schemas_t collection_schemas = get_schemas(collection_name);
auto schema_itr = collection_schemas.require_find(schema_name.value,
"No schema with this name exists");

templates_t collection_templates = get_templates(collection_name);
auto template_itr = collection_templates.require_find(template_id,
"No template with the specified id exists for the specified collection");

check_name_length(new_mutable_data);

template_data_t template_data = get_template_data(collection_name);
auto template_data_itr = template_data.find(template_id);

ATTRIBUTE_MAP deserialized_old_data;

if (template_data_itr != template_data.end()){
deserialized_old_data = deserialize(
template_data_itr->mutable_serialized_data,
schema_itr->format
);
}

action(
permission_level{get_self(), name("active")},
get_self(),
name("logsetdatatl"),
make_tuple(collection_name, schema_name, template_id, deserialized_old_data, new_mutable_data)
).send();

// If entry doesn't exist, then emplace entry
Copy link

Choose a reason for hiding this comment

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

This control flow has a problem where you could call it with empty new mutable data, and this empty data would then be emplaced. Doesn't break anything, but I think the following controlflow would be better and also clearer to understand (in pseudocode):

if no template data entry exists {
  check new_mutable_data must not be empty
  emplace
} else {
  if new_mutable_data is empty {
    erase
  } else {
    modify
  }
}

Copy link
Author

Choose a reason for hiding this comment

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

Added a condition for emplacing. It's a rather unique scenario & would likely only be executed by accident by users, but this just makes it a bit cleaner internally & more consistent with the entry modification functionality, while still not throwing an error to the user & breaking the transaction.

I'm leaning towards not having a check here to avoid breaking transactions when it's not critical / absolutely necessary.

    if (template_data_itr == template_data.end() && new_mutable_data.size() > 0){
        template_data.emplace(authorized_editor, [&](auto &_template_data) {
            _template_data.template_id = template_id;
            _template_data.schema_name = template_itr->schema_name;
            _template_data.mutable_serialized_data = serialize(new_mutable_data, schema_itr->format);
        });
    }

if (template_data_itr == template_data.end()){
template_data.emplace(authorized_editor, [&](auto &_template_data) {
_template_data.template_id = template_id;
_template_data.schema_name = schema_name;
_template_data.mutable_serialized_data = serialize(new_mutable_data, schema_itr->format);
});
}

// If entry exists && new_mutable_data is not empty, then modify entry
if (template_data_itr != template_data.end() && new_mutable_data.size() > 0){
template_data.modify(template_data_itr, authorized_editor, [&](auto &_template_data) {
_template_data.mutable_serialized_data = serialize(new_mutable_data, schema_itr->format);
});
}

// If entry exists && new_mutable_data is empty, then erase entry
if (template_data_itr != template_data.end() && new_mutable_data.size() == 0){
template_data.erase(template_data_itr);
}

}

/**
* This action is used to add a zero value asset to the quantities vector of owner in the balances table
Expand Down Expand Up @@ -1187,6 +1240,17 @@ ACTION atomicassets::logsetdata(
notify_collection_accounts(asset_itr->collection_name);
}

ACTION atomicassets::logsetdatatl(
name collection_name,
name schema_name,
int32_t template_id,
ATTRIBUTE_MAP old_data,
ATTRIBUTE_MAP new_data
) {
require_auth(get_self());

notify_collection_accounts(collection_name);
}

ACTION atomicassets::logbackasset(
name asset_owner,
Expand Down Expand Up @@ -1221,6 +1285,96 @@ ACTION atomicassets::logburnasset(
}


/**
* Function for creating a template, handling both the creation of normal templates & purely mutable templates
*/

void atomicassets::create_template(
name & authorized_creator,
name & collection_name,
name & schema_name,
bool transferable,
bool burnable,
uint32_t max_supply,
ATTRIBUTE_MAP & immutable_data,
ATTRIBUTE_MAP mutable_data
) {
require_auth(authorized_creator);

auto collection_itr = collections.require_find(collection_name.value,
"No collection with this name exists");

check_has_collection_auth(
authorized_creator,
collection_name,
"The creator is not authorized within the collection"
);

schemas_t collection_schemas = get_schemas(collection_name);
auto schema_itr = collection_schemas.require_find(schema_name.value,
"No schema with this name exists");

config_s current_config = config.get();
int32_t template_id = current_config.template_counter++;
config.set(current_config, get_self());

if (transferable == false){
Copy link

Choose a reason for hiding this comment

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

I'd prefer
check(burnable || transferable, "A template cannot be both non-transferable and non-burnable")

Copy link
Author

Choose a reason for hiding this comment

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

clean

check(burnable == true,
"A template cannot be both non-transferable and non-burnable");
}

templates_t collection_templates = get_templates(collection_name);

check_name_length(immutable_data);
collection_templates.emplace(authorized_creator, [&](auto &_template) {
_template.template_id = template_id;
_template.schema_name = schema_name;
_template.transferable = transferable;
_template.burnable = burnable;
_template.max_supply = max_supply;
_template.issued_supply = 0;
if (immutable_data.size() > 0){
_template.immutable_serialized_data = serialize(immutable_data, schema_itr->format);
}
});

action(
permission_level{get_self(), name("active")},
get_self(),
name("lognewtempl"),
make_tuple(
template_id,
authorized_creator,
collection_name,
schema_name,
transferable,
burnable,
max_supply,
immutable_data
)
).send();

if (mutable_data.size() > 0){
check_name_length(mutable_data);

template_data_t template_data = get_template_data(collection_name);
template_data.emplace(authorized_creator, [&](auto &_template_data) {
_template_data.template_id = template_id;
_template_data.schema_name = schema_name;
_template_data.mutable_serialized_data = serialize(mutable_data, schema_itr->format);
});

action(
permission_level{get_self(), name("active")},
get_self(),
name("logsetdatatl"),
make_tuple(collection_name, schema_name, template_id, mutable_data, mutable_data)
Copy link

Choose a reason for hiding this comment

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

old data in the log should be empty, not the mutable data as well.

).send();
}

}


/**
* Transfers need to be handled like this (as a function instead of an action), because when accepting an offer,
* we want each side of the offer to pay for their own scope. Because the recipient authorized the accept action,
Expand Down Expand Up @@ -1482,4 +1636,8 @@ atomicassets::schemas_t atomicassets::get_schemas(name collection_name) {

atomicassets::templates_t atomicassets::get_templates(name collection_name) {
return templates_t(get_self(), collection_name.value);
}

atomicassets::template_data_t atomicassets::get_template_data(name collection_name) {
return template_data_t(get_self(), collection_name.value);
}