From d3885ac67a119673f285de229587e0ecfdb7a62c Mon Sep 17 00:00:00 2001 From: KtorZ Date: Thu, 30 Jan 2025 15:45:50 +0100 Subject: [PATCH] prune orphan pair definitions after full blueprint generation. This is to avoid pruning a definition which may end up needed later on. The issue can be seen when definition to a Pair is used *before* another Map definitions that uses this same Pair. Before this commit, the Map definition would simply remove the definition generated for the Pair, since it would be pointless (and it is a lot easier to generate those pointless definition than trying to remember we are currently generating definition for a Map). So now, we defer the removal of the orphan definition to after all defnitions have been generated by basically looking at a dependency graph. I _could have_ used pet-graph on this to solve it similar to how we do package dependencies; but given that we only really need to that for pairs, the problem is relatively simple to solve (though cumbersome since we need to traverse all defintions). Fixes #1086. --- .../src/blueprint/definitions.rs | 155 +++++++++++++++++- crates/aiken-project/src/blueprint/schema.rs | 2 - ...ueprint__validator__tests__map_in_map.snap | 73 +++++++++ ...print__validator__tests__pair_in_pair.snap | 72 ++++++++ ...validator__tests__pair_used_after_map.snap | 75 +++++++++ ...alidator__tests__pair_used_before_map.snap | 75 +++++++++ .../aiken-project/src/blueprint/validator.rs | 71 +++++++- 7 files changed, 519 insertions(+), 4 deletions(-) create mode 100644 crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__map_in_map.snap create mode 100644 crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_in_pair.snap create mode 100644 crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_after_map.snap create mode 100644 crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_before_map.snap diff --git a/crates/aiken-project/src/blueprint/definitions.rs b/crates/aiken-project/src/blueprint/definitions.rs index efcd8569..7815556d 100644 --- a/crates/aiken-project/src/blueprint/definitions.rs +++ b/crates/aiken-project/src/blueprint/definitions.rs @@ -1,3 +1,10 @@ +use crate::{ + blueprint::{ + parameter::Parameter, + schema::{Data, Declaration, Items}, + }, + Annotated, Schema, +}; use aiken_lang::tipo::{pretty::resolve_alias, Type, TypeAliasAnnotation, TypeVar}; use itertools::Itertools; use serde::{ @@ -6,7 +13,7 @@ use serde::{ ser::{Serialize, SerializeStruct, Serializer}, }; use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap}, fmt::{self, Display}, ops::Deref, rc::Rc, @@ -88,6 +95,152 @@ impl Definitions { } } +impl Definitions> { + /// Remove orphan definitions. Such definitions can exist due to List of pairs being + /// transformed to Maps. As a consequence, we may generate temporary Pair definitions + /// which needs to get cleaned up later. + /// + /// Initially, we would clean those Pair definitions right-away, but this would cause + /// Pair definitions to be missing in some legit cases when the Pair is also used as a + /// standalone type. + pub fn prune_orphan_pairs(&mut self, parameters: Vec<&Parameter>) { + fn traverse_schema( + src: Reference, + schema: &Schema, + usage: &mut BTreeMap>, + ) { + match schema { + Schema::Unit + | Schema::Boolean + | Schema::Integer + | Schema::Bytes + | Schema::String => (), + Schema::Pair(left, right) => { + mark(src.clone(), left, usage, traverse_schema); + mark(src, right, usage, traverse_schema); + } + Schema::List(Items::One(item)) => { + mark(src, item, usage, traverse_schema); + } + Schema::List(Items::Many(items)) => { + items.iter().for_each(|item| { + mark(src.clone(), item, usage, traverse_schema); + }); + } + Schema::Data(data) => traverse_data(src, data, usage), + } + } + + fn traverse_data( + src: Reference, + data: &Data, + usage: &mut BTreeMap>, + ) { + match data { + Data::Opaque | Data::Integer | Data::Bytes => (), + Data::List(Items::One(item)) => { + mark(src, item, usage, traverse_data); + } + Data::List(Items::Many(items)) => { + items.iter().for_each(|item| { + mark(src.clone(), item, usage, traverse_data); + }); + } + Data::Map(keys, values) => { + mark(src.clone(), keys, usage, traverse_data); + mark(src, values, usage, traverse_data); + } + Data::AnyOf(items) => { + items.iter().for_each(|item| { + item.annotated.fields.iter().for_each(|field| { + mark(src.clone(), &field.annotated, usage, traverse_data); + }) + }); + } + } + } + + /// A mutually recursive function which works with either traverse_data or traverse_schema; + /// it is meant to peel the 'Declaration' and keep traversing if needed (when inline). + fn mark( + src: Reference, + declaration: &Declaration, + usage: &mut BTreeMap>, + mut traverse: F, + ) where + F: FnMut(Reference, &T, &mut BTreeMap>), + { + match declaration { + Declaration::Referenced(reference) => { + if let Some(dependencies) = usage.get_mut(reference) { + dependencies.insert(src); + } + } + Declaration::Inline(ref schema) => traverse(src, schema, usage), + } + } + + let mut usage: BTreeMap> = BTreeMap::new(); + + // 1. List all Pairs definitions + for (src, annotated) in self.inner.iter() { + if let Some(schema) = annotated.as_ref().map(|entry| &entry.annotated) { + if matches!(schema, Schema::Pair(_, _)) { + usage.insert(Reference::new(src), BTreeSet::new()); + } + } + } + + // 2. Mark those used in other definitions + for (src, annotated) in self.inner.iter() { + if let Some(schema) = annotated.as_ref().map(|entry| &entry.annotated) { + traverse_schema(Reference::new(src), schema, &mut usage) + } + } + + // 3. Mark also pairs definitions used in parameters / datums / redeemers + for (ix, param) in parameters.iter().enumerate() { + mark( + // NOTE: The name isn't important, so long as it doesn't clash with other typical + // references. If a definition is used in either of the parameter, it'll appear in + // its dependencies and won't ever be removed because parameters are considered + // always necessary. + Reference::new(&format!("__param^{ix}")), + ¶m.schema, + &mut usage, + traverse_schema, + ); + } + + // 4. Repeatedly remove pairs definitions that aren't used. We need doing this repeatedly + // because a Pair definition may only be used by an unused one; so as we prune the first + // unused definitions, new ones may become unused. + let mut last_len = usage.len(); + loop { + let mut unused = None; + for (k, v) in usage.iter() { + if v.is_empty() { + unused = Some(k.clone()); + } + } + + if let Some(k) = unused { + usage.remove(&k); + self.inner.remove(k.as_key().as_str()); + for (_, v) in usage.iter_mut() { + v.remove(&k); + } + } + + if usage.len() == last_len { + break; + } else { + last_len = usage.len(); + } + } + } +} + // ---------- Reference /// A URI pointer to an underlying data-type. diff --git a/crates/aiken-project/src/blueprint/schema.rs b/crates/aiken-project/src/blueprint/schema.rs index f8b68c2c..52b1b521 100644 --- a/crates/aiken-project/src/blueprint/schema.rs +++ b/crates/aiken-project/src/blueprint/schema.rs @@ -346,8 +346,6 @@ impl Annotated { annotated: Schema::Pair(left, right), .. }) => { - definitions.remove(&generic); - let left = left.map(|inner| match inner { Schema::Data(data) => data, _ => panic!("impossible: left inhabitant of pair isn't Data but: {inner:#?}"), diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__map_in_map.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__map_in_map.snap new file mode 100644 index 00000000..c87f4655 --- /dev/null +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__map_in_map.snap @@ -0,0 +1,73 @@ +--- +source: crates/aiken-project/src/blueprint/validator.rs +description: "Code:\n\npub type OuterMap =\n List>\n\npub type InnerMap =\n List>\n\nvalidator placeholder {\n spend(_datum: Option, _redeemer: OuterMap, _utxo: Data, _self: Data,) {\n True\n }\n}\n" +--- +{ + "title": "test_module.placeholder.spend", + "datum": { + "title": "_datum", + "schema": { + "$ref": "#/definitions/Void" + } + }, + "redeemer": { + "title": "_redeemer", + "schema": { + "$ref": "#/definitions/OuterMap" + } + }, + "compiledCode": "", + "hash": "", + "definitions": { + "Bool": { + "title": "Bool", + "anyOf": [ + { + "title": "False", + "dataType": "constructor", + "index": 0, + "fields": [] + }, + { + "title": "True", + "dataType": "constructor", + "index": 1, + "fields": [] + } + ] + }, + "InnerMap": { + "title": "InnerMap", + "dataType": "map", + "keys": { + "$ref": "#/definitions/Int" + }, + "values": { + "$ref": "#/definitions/Bool" + } + }, + "Int": { + "dataType": "integer" + }, + "OuterMap": { + "title": "OuterMap", + "dataType": "map", + "keys": { + "$ref": "#/definitions/Int" + }, + "values": { + "$ref": "#/definitions/InnerMap" + } + }, + "Void": { + "title": "Unit", + "anyOf": [ + { + "dataType": "constructor", + "index": 0, + "fields": [] + } + ] + } + } +} diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_in_pair.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_in_pair.snap new file mode 100644 index 00000000..e5cb9c86 --- /dev/null +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_in_pair.snap @@ -0,0 +1,72 @@ +--- +source: crates/aiken-project/src/blueprint/validator.rs +description: "Code:\n\npub type MyPair =\n Pair\n\npub type InnerPair =\n Pair\n\nvalidator placeholder {\n spend(_datum: Option, _redeemer: List, _utxo: Data, _self: Data,) {\n True\n }\n}\n" +--- +{ + "title": "test_module.placeholder.spend", + "datum": { + "title": "_datum", + "schema": { + "$ref": "#/definitions/Void" + } + }, + "redeemer": { + "title": "_redeemer", + "schema": { + "$ref": "#/definitions/List$MyPair" + } + }, + "compiledCode": "", + "hash": "", + "definitions": { + "Bool": { + "title": "Bool", + "anyOf": [ + { + "title": "False", + "dataType": "constructor", + "index": 0, + "fields": [] + }, + { + "title": "True", + "dataType": "constructor", + "index": 1, + "fields": [] + } + ] + }, + "InnerPair": { + "title": "InnerPair", + "dataType": "#pair", + "left": { + "$ref": "#/definitions/Int" + }, + "right": { + "$ref": "#/definitions/Bool" + } + }, + "Int": { + "dataType": "integer" + }, + "List$MyPair": { + "dataType": "map", + "keys": { + "$ref": "#/definitions/Int" + }, + "values": { + "$ref": "#/definitions/InnerPair" + } + }, + "Void": { + "title": "Unit", + "anyOf": [ + { + "dataType": "constructor", + "index": 0, + "fields": [] + } + ] + } + } +} diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_after_map.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_after_map.snap new file mode 100644 index 00000000..77ad17ab --- /dev/null +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_after_map.snap @@ -0,0 +1,75 @@ +--- +source: crates/aiken-project/src/blueprint/validator.rs +description: "Code:\n\npub type MyPair =\n Pair\n\npub type MyDatum {\n pairs: List,\n pair: MyPair,\n}\n\nvalidator placeholder {\n spend(_datum: Option, _redeemer: Void, _utxo: Data, _self: Data,) {\n True\n }\n}\n" +--- +{ + "title": "test_module.placeholder.spend", + "datum": { + "title": "_datum", + "schema": { + "$ref": "#/definitions/test_module~1MyDatum" + } + }, + "redeemer": { + "title": "_redeemer", + "schema": { + "$ref": "#/definitions/Void" + } + }, + "compiledCode": "", + "hash": "", + "definitions": { + "Int": { + "dataType": "integer" + }, + "List$MyPair": { + "dataType": "map", + "keys": { + "$ref": "#/definitions/Int" + }, + "values": { + "$ref": "#/definitions/Int" + } + }, + "MyPair": { + "title": "MyPair", + "dataType": "#pair", + "left": { + "$ref": "#/definitions/Int" + }, + "right": { + "$ref": "#/definitions/Int" + } + }, + "Void": { + "title": "Unit", + "anyOf": [ + { + "dataType": "constructor", + "index": 0, + "fields": [] + } + ] + }, + "test_module/MyDatum": { + "title": "MyDatum", + "anyOf": [ + { + "title": "MyDatum", + "dataType": "constructor", + "index": 0, + "fields": [ + { + "title": "pairs", + "$ref": "#/definitions/List$MyPair" + }, + { + "title": "pair", + "$ref": "#/definitions/MyPair" + } + ] + } + ] + } + } +} diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_before_map.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_before_map.snap new file mode 100644 index 00000000..3bb64dbc --- /dev/null +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__pair_used_before_map.snap @@ -0,0 +1,75 @@ +--- +source: crates/aiken-project/src/blueprint/validator.rs +description: "Code:\n\npub type MyPair =\n Pair\n\npub type MyDatum {\n pair: MyPair,\n pairs: List,\n}\n\nvalidator placeholder {\n spend(_datum: Option, _redeemer: Void, _utxo: Data, _self: Data,) {\n True\n }\n}\n" +--- +{ + "title": "test_module.placeholder.spend", + "datum": { + "title": "_datum", + "schema": { + "$ref": "#/definitions/test_module~1MyDatum" + } + }, + "redeemer": { + "title": "_redeemer", + "schema": { + "$ref": "#/definitions/Void" + } + }, + "compiledCode": "", + "hash": "", + "definitions": { + "Int": { + "dataType": "integer" + }, + "List$MyPair": { + "dataType": "map", + "keys": { + "$ref": "#/definitions/Int" + }, + "values": { + "$ref": "#/definitions/Int" + } + }, + "MyPair": { + "title": "MyPair", + "dataType": "#pair", + "left": { + "$ref": "#/definitions/Int" + }, + "right": { + "$ref": "#/definitions/Int" + } + }, + "Void": { + "title": "Unit", + "anyOf": [ + { + "dataType": "constructor", + "index": 0, + "fields": [] + } + ] + }, + "test_module/MyDatum": { + "title": "MyDatum", + "anyOf": [ + { + "title": "MyDatum", + "dataType": "constructor", + "index": 0, + "fields": [ + { + "title": "pair", + "$ref": "#/definitions/MyPair" + }, + { + "title": "pairs", + "$ref": "#/definitions/List$MyPair" + } + ] + } + ] + } + } +} diff --git a/crates/aiken-project/src/blueprint/validator.rs b/crates/aiken-project/src/blueprint/validator.rs index e433df7e..b51665d1 100644 --- a/crates/aiken-project/src/blueprint/validator.rs +++ b/crates/aiken-project/src/blueprint/validator.rs @@ -122,7 +122,7 @@ impl Validator { ), }) }) - .collect::>()?; + .collect::, _>>()?; let (datum, redeemer) = if func.name == well_known::VALIDATOR_ELSE { (None, None) @@ -202,6 +202,14 @@ impl Validator { schema: Declaration::Inline(Box::new(Schema::Data(Data::Opaque))), })); + definitions.prune_orphan_pairs( + parameters + .iter() + .chain(redeemer.as_ref().map(|x| vec![x]).unwrap_or_default()) + .chain(datum.as_ref().map(|x| vec![x]).unwrap_or_default()) + .collect::>(), + ); + Ok(Validator { title: format!("{}.{}.{}", &module.name, &def.name, &func.name,), description: func.doc.clone(), @@ -736,6 +744,67 @@ mod tests { ); } + #[test] + fn pair_used_after_map() { + assert_validator!( + r#" + pub type MyPair = + Pair + + pub type MyDatum { + pairs: List, + pair: MyPair, + } + + validator placeholder { + spend(_datum: Option, _redeemer: Void, _utxo: Data, _self: Data,) { + True + } + } + "# + ); + } + + #[test] + fn pair_used_before_map() { + assert_validator!( + r#" + pub type MyPair = + Pair + + pub type MyDatum { + pair: MyPair, + pairs: List, + } + + validator placeholder { + spend(_datum: Option, _redeemer: Void, _utxo: Data, _self: Data,) { + True + } + } + "# + ); + } + + #[test] + fn map_in_map() { + assert_validator!( + r#" + pub type OuterMap = + List> + + pub type InnerMap = + List> + + validator placeholder { + spend(_datum: Option, _redeemer: OuterMap, _utxo: Data, _self: Data,) { + True + } + } + "# + ); + } + #[test] fn else_redeemer() { assert_validator!(