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

Make sizes configurable #98

Open
jannikluhn opened this issue Nov 18, 2019 · 2 comments
Open

Make sizes configurable #98

jannikluhn opened this issue Nov 18, 2019 · 2 comments

Comments

@jannikluhn
Copy link
Contributor

What is wrong

Sizes of lists, vectors, bitlists, and bitvectors are defined when the corresponding sedes object is created. For sedes objects created in some module, this corresponds to the import time of the module. However, some of these sizes we would like to extract from a configuration loaded only at a later point in time. See also ethereum/trinity#786

How can it be fixed?

Make sizes not part of the sedes object. Instead, require a local config dictionary parameter for all serialization, deserialization, and hash tree root computation functions:

BaseSedes.serialize(value, config)
BaseSedes.deserialize(value, config):
BaseSedes.get_hash_tree_root(value, config)

encode(value, sedes=None, config=None)
decode(value, sedes=None, config=None)
get_hash_tree_root(value, sedes=None, config=None)

Depending on the object type, the config dictionary should have the following structure:

list/vector/bitlist/bitvector_config = {
    "size": size_or_max_size_or_bit_num,
    "element_config": {  # optional, if the element requires a config
        ...
    }
}

container_config = (None, None, config_for_field_3, None, config_for_field_5)

Hashables should receive a *global" config dictionary at instantiation:

global_ssz_config = {
    BeaconState: {
        "block_roots": 10,
        "eth1_data_votes": 5,
        ...
    },
    PendingAttestation: {
        "aggregation_bits": 123,
    },
    ValidatorList: 10  # if we define a class ValidatorList deriving from HashableList
    ...
}

Note that this maps the defined classes to the size config values of their fields/elements. I hope this doesn't lead to import cycles.

decode, encode, and get_hash_tree_root should create the (nested) local config object from the corresponding (flat) global config. The config should be immutable (a PMap?) so that hashables can safely do their caching.

Some sizes should not be configurable (e.g., bytes32, signatures, etc.). For these, we should create custom subclasses of Vector/BitVector that do not expect a configuration parameter.

Concerns

  • The config is very rigid, it's hard to add additional parameters (e.g. removing certain fields from a container). I'm not sure if this is necessary though.
  • The distinction between local and global config could be confusing and using the API without hashables would be less straightforward.
@ralexstokes
Copy link
Member

Note that this maps the defined classes to the size config values of their fields/elements. I hope this doesn't lead to import cycles.

the straightforward approach here is to "stringly type" the class name. which is not preferable w/ an eye towards type safety but if we have to break an import cycle then it may suffice.

Hashables should receive a *global" config dictionary at instantiation:

while it could be more verbose, why do we need the global config?

instead, every type gets a "local" config and then we have some canonical keys for properties they expect in the config, like e.g. MaxLength or something

@jannikluhn
Copy link
Contributor Author

while it could be more verbose, why do we need the global config?

instead, every type gets a "local" config and then we have some canonical keys for properties they expect in the config, like e.g. MaxLength or something

yeah, I was mostly worried about usability, because of all the nesting going and potential duplication of constants. Which is not a problem if it is auto-generated from something simpler. But maybe the global config is unnecessary, at least inside of py-ssz (we could have something similar in Trinity).

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

No branches or pull requests

2 participants