Builder bug

[njgheorghita]

What was wrong?

A couple minor bugs in the builder needed squashing. Validation of TxReceipt wasn't accounting for the case when a contract's creation address is None accurately.

How was it fixed?

Squashed

Cute Animal Picture

[njgheorghita]

@pipermerriam It's more so that if somebody wants to use the validate_deployments_tx_receipt util and enforce that the deployment data is present and matches the on-chain complement data (eg. trigger an exception if the deployment data in the manifest is insufficient or incorrect).

Though, it's a pretty horrible name - and i'm not sure of it's usefulness. The alternative would be making the validation util more lazy, and simple verifying that if there is deployment data in the manifest, then assert that it matches what's on-chain - otherwise carry on. So basically only raise an exception if the deployment data is present and incorrect, not if it's missing.

[pipermerriam]

So basically only raise an exception if the deployment data is incorrect, not if it's missing.

This is probably worthwhile. Maybe even just split this into two independent validation mechanism but I can't quite figure out how to name them well.

• validate_verifiable_deployment_data
  • validates data is present
  • validates data is accurate
• soft_validate_deployment_data
  • always passes if data is missing
  • verifies accuracy if it is present

I think the later is actually a foot-gun so maybe we just only implement the hard validation and wait a bit to figure out the right way to do less-strict validation?

[njgheorghita]

Well, as it currently stands -validate_deployments_tx_receipt is run on all deployments. So for instance, if we were to implement the "hard" validation - manifests with only an address in their deployment data would fail. It makes more sense to me to implement soft_validate_deployment_data now and wait on the "hard" validation. That way we could continue to run every manifest's deployment data through this validation, and not worry about it blowing up if somebody has insufficient data (the spec does not require that the tx, block, ... are included a deployment's data.

[pipermerriam]

Mostly I'm worried about an API that gives users a false sense of security. I think the current approach is fine since it defaults to being strict but allows for non-strict validation. Maybe leave as-is for now and open an issues to track this ambiguousness on what the right API is.