Add new API Version to validate when setting fields not defined in th…

…e schema (#4894) * build(deps): bump chrono from 0.4.26 to 0.4.31 (#4876) Bumps [chrono](https://github.com/chronotope/chrono) from 0.4.26 to 0.4.31. - [Release notes](https://github.com/chronotope/chrono/releases) - [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md) - [Commits](chronotope/chrono@v0.4.26...v0.4.31) --- updated-dependencies: - dependency-name: chrono dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump webpki from 0.22.0 to 0.22.1 (#4857) Bumps [webpki](https://github.com/briansmith/webpki) from 0.22.0 to 0.22.1. - [Commits](https://github.com/briansmith/webpki/commits) --- updated-dependencies: - dependency-name: webpki dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * runtime: only include valid fields in entity for store_set * graph, runtime: add new apiVersion to validate fields not defined in the schema * graph: update tests for setting invalid field * tests: add runner tests for undefined field setting validation in apiVersion 0.0.8 * graph: add check_invalid_fields method to HostExports --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
graphprotocol · Oct 22, 2023 · a963f19 · a963f19
1 parent fb2184d
commit a963f19
Show file tree

Hide file tree

Showing 18 changed files with 332 additions and 32 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/NEWS.md b/NEWS.md
@@ -12,7 +12,7 @@
 - **Initialization handler** - A new block handler filter `once` for `ethereum` data sources which enables subgraph developers to create a handle which will be called only once before all other handlers run. This configuration allows the subgraph to use the handler as an initialization handler, performing specific tasks at the start of indexing. [(#4725)](https://github.com/graphprotocol/graph-node/pull/4725)
 - **DataSourceContext in manifest** - `DataSourceContext` in Manifest - DataSourceContext can now be defined in the subgraph manifest. It's a free-form map accessible from the mapping. This feature is useful for templating chain-specific data in subgraphs that use the same codebase across multiple chains.[(#4848)](https://github.com/graphprotocol/graph-node/pull/4848)
 - `graph-node` version in index node API - The Index Node API now features a new query, Version, which can be used to query the current graph-node version and commit. [(#4852)](https://github.com/graphprotocol/graph-node/pull/4852)
-- Added subgraph status to index node API [(#4779)](https://github.com/graphprotocol/graph-node/pull/4779)
+- Added a '`paused`' field to Index Node API, a boolean indicating the subgraph’s pause status. [(#4779)](https://github.com/graphprotocol/graph-node/pull/4779)
 - Proof of Indexing logs now include block number [(#4798)](https://github.com/graphprotocol/graph-node/pull/4798)
 - `subgraph_features` table now tracks details about handlers used in a subgraph [(#4820)](https://github.com/graphprotocol/graph-node/pull/4820)
 - Configurable SSL for Postgres in Dockerfile -  ssl-mode for Postgres can now be configured via the connection string when deploying through Docker, offering enhanced flexibility in database security settings.[(#4840)](https://github.com/graphprotocol/graph-node/pull/4840)
@@ -43,7 +43,7 @@ Not Relevant
 * Update docker-compose.yml by @computeronix in https://github.com/graphprotocol/graph-node/pull/4844
  -->
 
-**Full Changelog**: https://github.com/graphprotocol/graph-node/compare/v0.32.0...e253ee14cda2d8456a86ae8f4e3f74a1a7979953
+**Full Changelog**: https://github.com/graphprotocol/graph-node/compare/v0.33.0...e253ee14cda2d8456a86ae8f4e3f74a1a7979953
 
 ## v0.32.0
 

diff --git a/graph/src/data/store/mod.rs b/graph/src/data/store/mod.rs
@@ -657,9 +657,6 @@ impl<E, T: IntoIterator<Item = Result<(Word, Value), E>>> TryIntoEntityIterator<
 
 #[derive(Debug, Error, PartialEq, Eq, Clone)]
 pub enum EntityValidationError {
-    #[error("The provided entity has fields not defined in the schema for entity `{entity}`")]
-    FieldsNotDefined { entity: String },
-
     #[error("Entity {entity}[{id}]: unknown entity type `{entity}`")]
     UnknownEntityType { entity: String, id: String },
 
@@ -918,14 +915,6 @@ impl Entity {
             }
         })?;
 
-        for field in self.0.atoms() {
-            if !key.entity_type.has_field(field) {
-                return Err(EntityValidationError::FieldsNotDefined {
-                    entity: key.entity_type.to_string(),
-                });
-            }
-        }
-
         for field in &object_type.fields {
             let is_derived = field.is_derived();
             match (self.get(&field.name), is_derived) {

diff --git a/graph/src/data/subgraph/api_version.rs b/graph/src/data/subgraph/api_version.rs
@@ -15,6 +15,9 @@ pub const API_VERSION_0_0_6: Version = Version::new(0, 0, 6);
 /// Enables event handlers to require transaction receipts in the runtime.
 pub const API_VERSION_0_0_7: Version = Version::new(0, 0, 7);
 
+/// Enables validation for fields that doesnt exist in the schema for an entity.
+pub const API_VERSION_0_0_8: Version = Version::new(0, 0, 8);
+
 /// Before this check was introduced, there were already subgraphs in the wild with spec version
 /// 0.0.3, due to confusion with the api version. To avoid breaking those, we accept 0.0.3 though it
 /// doesn't exist.

diff --git a/graph/src/env/mappings.rs b/graph/src/env/mappings.rs
@@ -16,7 +16,7 @@ pub struct EnvVarsMapping {
     /// kilobytes). The default value is 10 megabytes.
     pub entity_cache_size: usize,
     /// Set by the environment variable `GRAPH_MAX_API_VERSION`. The default
-    /// value is `0.0.7`.
+    /// value is `0.0.8`.
     pub max_api_version: Version,
     /// Set by the environment variable `GRAPH_MAPPING_HANDLER_TIMEOUT`
     /// (expressed in seconds). No default is provided.
@@ -93,7 +93,7 @@ pub struct InnerMappingHandlers {
     entity_cache_dead_weight: EnvVarBoolean,
     #[envconfig(from = "GRAPH_ENTITY_CACHE_SIZE", default = "10000")]
     entity_cache_size_in_kb: usize,
-    #[envconfig(from = "GRAPH_MAX_API_VERSION", default = "0.0.7")]
+    #[envconfig(from = "GRAPH_MAX_API_VERSION", default = "0.0.8")]
     max_api_version: Version,
     #[envconfig(from = "GRAPH_MAPPING_HANDLER_TIMEOUT")]
     mapping_handler_timeout_in_secs: Option<u64>,

diff --git a/graph/src/schema/entity_type.rs b/graph/src/schema/entity_type.rs
@@ -23,7 +23,7 @@ use super::{input_schema::POI_OBJECT, EntityKey, InputSchema};
 #[derive(Clone)]
 pub struct EntityType {
     schema: InputSchema,
-    atom: Atom,
+    pub atom: Atom,
 }
 
 impl EntityType {

diff --git a/graph/src/schema/input_schema.rs b/graph/src/schema/input_schema.rs
@@ -467,6 +467,14 @@ impl InputSchema {
                 )
             })
     }
+    pub fn has_field_with_name(&self, entity_type: &EntityType, field: &str) -> bool {
+        let field = self.inner.pool.lookup(field);
+
+        match field {
+            Some(field) => self.has_field(entity_type.atom, field),
+            None => false,
+        }
+    }
 }
 
 /// Create a new pool that contains the names of all the types defined

diff --git a/runtime/test/src/test.rs b/runtime/test/src/test.rs
@@ -1226,8 +1226,13 @@ struct Host {
 }
 
 impl Host {
-    async fn new(schema: &str, deployment_hash: &str, wasm_file: &str) -> Host {
-        let version = ENV_VARS.mappings.max_api_version.clone();
+    async fn new(
+        schema: &str,
+        deployment_hash: &str,
+        wasm_file: &str,
+        api_version: Option<Version>,
+    ) -> Host {
+        let version = api_version.unwrap_or(ENV_VARS.mappings.max_api_version.clone());
         let wasm_file = wasm_file_path(wasm_file, API_VERSION_0_0_5);
 
         let ds = mock_data_source(&wasm_file, version.clone());
@@ -1325,7 +1330,7 @@ async fn test_store_set_id() {
         name: String,
     }";
 
-    let mut host = Host::new(schema, "hostStoreSetId", "boolean.wasm").await;
+    let mut host = Host::new(schema, "hostStoreSetId", "boolean.wasm", None).await;
 
     host.store_set(USER, UID, vec![("id", "u1"), ("name", "user1")])
         .expect("setting with same id works");
@@ -1428,7 +1433,13 @@ async fn test_store_set_invalid_fields() {
         test2: String
     }";
 
-    let mut host = Host::new(schema, "hostStoreSetInvalidFields", "boolean.wasm").await;
+    let mut host = Host::new(
+        schema,
+        "hostStoreSetInvalidFields",
+        "boolean.wasm",
+        Some(API_VERSION_0_0_8),
+    )
+    .await;
 
     host.store_set(USER, UID, vec![("id", "u1"), ("name", "user1")])
         .unwrap();
@@ -1451,8 +1462,7 @@ async fn test_store_set_invalid_fields() {
     // So we just check the string contains them
     let err_string = err.to_string();
     dbg!(err_string.as_str());
-    assert!(err_string
-        .contains("The provided entity has fields not defined in the schema for entity `User`"));
+    assert!(err_string.contains("Attempted to set undefined fields [test, test2] for the entity type `User`. Make sure those fields are defined in the schema."));
 
     let err = host
         .store_set(
@@ -1463,8 +1473,30 @@ async fn test_store_set_invalid_fields() {
         .err()
         .unwrap();
 
-    err_says(
-        err,
-        "Unknown key `test3`. It probably is not part of the schema",
+    err_says(err, "Attempted to set undefined fields [test3] for the entity type `User`. Make sure those fields are defined in the schema.");
+
+    // For apiVersion below 0.0.8, we should not error out
+    let mut host2 = Host::new(
+        schema,
+        "hostStoreSetInvalidFields",
+        "boolean.wasm",
+        Some(API_VERSION_0_0_7),
     )
+    .await;
+
+    let err_is_none = host2
+        .store_set(
+            USER,
+            UID,
+            vec![
+                ("id", "u1"),
+                ("name", "user1"),
+                ("test", "invalid_field"),
+                ("test2", "invalid_field"),
+            ],
+        )
+        .err()
+        .is_none();
+
+    assert!(err_is_none);
 }
diff --git a/runtime/wasm/src/host_exports.rs b/runtime/wasm/src/host_exports.rs
@@ -4,6 +4,7 @@ use std::ops::Deref;
 use std::str::FromStr;
 use std::time::{Duration, Instant};
 
+use graph::data::subgraph::API_VERSION_0_0_8;
 use graph::data::value::Word;
 
 use graph::schema::EntityType;
@@ -151,6 +152,54 @@ impl<C: Blockchain> HostExports<C> {
         )))
     }
 
+    fn check_invalid_fields(
+        &self,
+        api_version: Version,
+        data: &HashMap<Word, Value>,
+        state: &BlockState<C>,
+        entity_type: &EntityType,
+    ) -> Result<(), HostExportError> {
+        if api_version >= API_VERSION_0_0_8 {
+            let has_invalid_fields = data.iter().any(|(field_name, _)| {
+                !state
+                    .entity_cache
+                    .schema
+                    .has_field_with_name(entity_type, &field_name)
+            });
+
+            if has_invalid_fields {
+                let mut invalid_fields: Vec<Word> = data
+                    .iter()
+                    .filter_map(|(field_name, _)| {
+                        if !state
+                            .entity_cache
+                            .schema
+                            .has_field_with_name(entity_type, &field_name)
+                        {
+                            Some(field_name.clone())
+                        } else {
+                            None
+                        }
+                    })
+                    .collect();
+
+                invalid_fields.sort();
+
+                return Err(HostExportError::Deterministic(anyhow!(
+                    "Attempted to set undefined fields [{}] for the entity type `{}`. Make sure those fields are defined in the schema.",
+                    invalid_fields
+                        .iter()
+                        .map(|f| f.as_str())
+                        .collect::<Vec<_>>()
+                        .join(", "),
+                    entity_type
+                )));
+            }
+        }
+
+        Ok(())
+    }
+
     pub(crate) fn store_set(
         &self,
         logger: &Logger,
@@ -198,9 +247,19 @@ impl<C: Blockchain> HostExports<C> {
             }
         }
 
+        self.check_invalid_fields(self.api_version.clone(), &data, state, &key.entity_type)?;
+
+        // Filter out fields that are not in the schema
+        let filtered_entity_data = data.into_iter().filter(|(field_name, _)| {
+            state
+                .entity_cache
+                .schema
+                .has_field_with_name(&key.entity_type, field_name)
+        });
+
         let entity = state
             .entity_cache
-            .make_entity(data.into_iter().map(|(key, value)| (key, value)))
+            .make_entity(filtered_entity_data)
             .map_err(|e| HostExportError::Deterministic(anyhow!(e)))?;
 
         let poi_section = stopwatch.start_section("host_export_store_set__proof_of_indexing");

diff --git a/tests/runner-tests/api-version/abis/Contract.abi b/tests/runner-tests/api-version/abis/Contract.abi
@@ -0,0 +1,15 @@
+[
+    {
+        "anonymous": false,
+        "inputs": [
+            {
+                "indexed": false,
+                "internalType": "string",
+                "name": "testCommand",
+                "type": "string"
+            }
+        ],
+        "name": "TestEvent",
+        "type": "event"
+    }
+]
diff --git a/tests/runner-tests/api-version/data.0.0.7.json b/tests/runner-tests/api-version/data.0.0.7.json
@@ -0,0 +1,3 @@
+{
+  "apiVersion": "0.0.7"
+}
diff --git a/tests/runner-tests/api-version/data.0.0.8.json b/tests/runner-tests/api-version/data.0.0.8.json
@@ -0,0 +1,3 @@
+{
+  "apiVersion": "0.0.8"
+}
diff --git a/tests/runner-tests/api-version/package.json b/tests/runner-tests/api-version/package.json
@@ -0,0 +1,29 @@
+{
+  "name": "api-version",
+  "version": "0.1.0",
+  "scripts": {
+    "build-contracts": "../../common/build-contracts.sh",
+    "codegen": "graph codegen --skip-migrations",
+    "test": "yarn build-contracts && truffle test --compile-none --network test",
+    "create:test": "graph create test/api-version --node $GRAPH_NODE_ADMIN_URI",
+    "prepare:0-0-7": "mustache data.0.0.7.json subgraph.template.yaml > subgraph.yaml",
+    "prepare:0-0-8": "mustache data.0.0.8.json  subgraph.template.yaml > subgraph.yaml",
+    "deploy:test-0-0-7": "yarn prepare:0-0-7 && graph deploy test/api-version-0-0-7 --version-label 0.0.7 --ipfs $IPFS_URI --node $GRAPH_NODE_ADMIN_URI",
+    "deploy:test-0-0-8": "yarn prepare:0-0-8 && graph deploy test/api-version-0-0-8 --version-label 0.0.8 --ipfs $IPFS_URI --node $GRAPH_NODE_ADMIN_URI"
+  },
+  "devDependencies": {
+    "@graphprotocol/graph-cli": "0.53.0",
+    "@graphprotocol/graph-ts": "0.31.0",
+    "solc": "^0.8.2"
+  },
+  "dependencies": {
+    "@truffle/contract": "^4.3",
+    "@truffle/hdwallet-provider": "^1.2",
+    "apollo-fetch": "^0.7.0",
+    "babel-polyfill": "^6.26.0",
+    "babel-register": "^6.26.0",
+    "gluegun": "^4.6.1",
+    "mustache": "^4.2.0",
+    "truffle": "^5.2"
+  }
+}
diff --git a/tests/runner-tests/api-version/schema.graphql b/tests/runner-tests/api-version/schema.graphql
@@ -0,0 +1,4 @@
+type TestResult @entity {
+  id: ID!
+  message: String!
+}
diff --git a/tests/runner-tests/api-version/src/mapping.ts b/tests/runner-tests/api-version/src/mapping.ts
@@ -0,0 +1,15 @@
+import { Entity, Value, store } from "@graphprotocol/graph-ts";
+import { TestEvent } from "../generated/Contract/Contract";
+import { TestResult } from "../generated/schema";
+
+export function handleTestEvent(event: TestEvent): void {
+  let testResult = new TestResult(event.params.testCommand);
+  testResult.message = event.params.testCommand;
+  let testResultEntity = testResult as Entity;
+  testResultEntity.set(
+    "invalid_field",
+    Value.fromString("This is an invalid field"),
+  );
+  store.set("TestResult", testResult.id, testResult);
+  testResult.save();
+}
diff --git a/tests/runner-tests/api-version/subgraph.template.yaml b/tests/runner-tests/api-version/subgraph.template.yaml
@@ -0,0 +1,23 @@
+specVersion: 0.0.4
+schema:
+  file: ./schema.graphql
+dataSources:
+  - kind: ethereum/contract
+    name: Contract
+    network: test
+    source:
+      address: "0x0000000000000000000000000000000000000000"
+      abi: Contract
+    mapping:
+      kind: ethereum/events
+      apiVersion: {{apiVersion}}
+      language: wasm/assemblyscript
+      abis:
+        - name: Contract
+          file: ./abis/Contract.abi
+      entities:
+        - Call
+      eventHandlers:
+        - event: TestEvent(string)
+          handler: handleTestEvent
+      file: ./src/mapping.ts