Incorrect/Undefined behavior when populus writes to configuration values under a `$ref`

[pipermerriam]

• Populus Version: 1.6.9
• OS: all

What was wrong?

Populus is not correctly handling the writing of configuration values into $ref based configuration sections.

Given the following configuration:

{
  "foo": {"$ref": "bar"},
  "bar": {"baz": 3},
}

When populus reads this, what it sees is:

{
  "foo": {"baz": 3},
  "bar": {"baz": 3},
}

The error occurs when a configuration value is written to a key under foo. Currently, if you were to set foo.test = 3 this is the result:

{
  "foo": {
    "$ref": "bar",
    "test": 3
  },
  "bar": {"baz": 3},
}

This should result in an invalid configuration once #283 has been addressed.

How can it be fixed?

Fixing this is going to require extending the configuration API as the current API does not have a convention to gracefully handle the case where a configuration value is written into a $ref. Since $ref values are designed to be shared resources across the config file, it doesn't make sense to write these values into the target of the $ref.

Instead we need to introduce a new API, $extends.

When the configuration file encounters an {"$extends": "path.to.section", ...} it should perform a deep merge of the local configuration and the configuration found under path.to.section.

The result is that when populus encounters the following configuration:

{
  "foo": {
    "$extends": "bar",
    "test": 3
  },
  "bar": {"baz": 3},
}

What it actually sees is:

{
  "foo": {
    "baz": 3,
    "test": 3
  },
  "bar": {"baz": 3},
}

[fubuloubu]

I was looking into using YAML for this exact reason: to be able to reference or extend other keys.

I'm not sure of the downsides of using YAML vs JSON for Populus, but currently I believe the JSON is just used as config files which makes it a great candidate for YAML instead.

I was reading through this post to get an idea of what this might look like.

Let me know what you think about this suggestion.

P.S. Could this be used to de-duplicate the runtime section of the bytecode from the bytecode itself?

[fubuloubu]

You can also extend YAML syntax: https://pyyaml.org/wiki/PyYAMLDocumentation

I was thinking something like this for referencing contract addresses as deployment parameters in other contracts:

foo: &foo !deploy # Use &foo to reference for other deployments
    # Everything under foo is an argument
    # Deploy foo with stuff: 'Whatever'
    stuff: Whatever
bar: !deploy # Not used elsewhere, so no reference required
    # Deploy bar with stuff: 'Don't worry about it', foo_address: address from deploying foo
    stuff: Don't worry about it
    foo_address: *foo # Gets address from deploying foo
baz: !deploy # Contract with no args for deployment (default if not specified in config)

Then we can register a new constructor (referencing a Deployment object) by add this to the YAML Load code:

import yaml

class Deployment(object):
    def __init__(self, args=None):
        self.args = args

    def get_address(self):
        if not hasattr(self, 'address'):
            self.address = 'Address from Deployment'
        return self.address

    def __repr__(self):
        return self.get_address()

def get_deployment(loader, node):
    if node.value:
        args = loader.construct_mapping(node)
        return Deployment(args=args)
    else:
        return Deployment()

# Register the tag handler
yaml.add_constructor('!deploy', get_deployment)

config = yaml.load(config_file_contents)

[pipermerriam]

Could this be used to de-duplicate the runtime section of the bytecode from the bytecode itself?

Not exactly.

• runtime is always a subset of the bytecode but it's not trivial to extract it.
• bytecode is always a superset of the runtime but the extra stuff is not always the same, nor can it be computed (because things like the constructor function are part of that extra stuff).

You really do need both assuming you want to both deploy contracts and be able to verify existing deployments.

[pipermerriam]

Let me know what you think about this suggestion.

I have a plan for what I hope is the final major overhaul to populus configuration. I hate the current state of things and it's proven the wrong solution. $ref has turned out to be a can-of-worms worse than the duplication that it was meant to fix. Upgrading configs has been a huge pain.

Current path forward look like it's going to retain the ability to control most things through configuration but to drastically reduce the size of the necessary config file. Exactly how this is going to implemented is still in flux but it's the top thing on the list because without fixing it, all the other things that need to be fixed are way more complicated because of the necessary config upgrades that go with them.