Skip to content

Commit

Permalink
Add new API Version to validate when setting fields not defined in th…
Browse files Browse the repository at this point in the history
…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] <support@github.com>
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] <support@github.com>
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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
incrypto32 and dependabot[bot] committed Oct 17, 2023
1 parent bad3223 commit e12c79c
Show file tree
Hide file tree
Showing 18 changed files with 336 additions and 47 deletions.
24 changes: 6 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bytes = "1.0.1"
cid = "0.10.1"
diesel = { version = "1.4.8", features = ["postgres", "serde_json", "numeric", "r2d2", "chrono"] }
diesel_derives = "1.4"
chrono = "0.4.25"
chrono = "0.4.31"
envconfig = "0.10.0"
Inflector = "0.11.3"
isatty = "0.1.9"
Expand Down
11 changes: 0 additions & 11 deletions graph/src/data/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,9 +663,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 },

Expand Down Expand Up @@ -928,14 +925,6 @@ impl Entity {
}
})?;

for field in self.0.atoms() {
if !schema.has_field(&key.entity_type, field) {
return Err(EntityValidationError::FieldsNotDefined {
entity: key.entity_type.clone().into_string(),
});
}
}

for field in &object_type.fields {
let is_derived = field.is_derived();
match (self.get(&field.name), is_derived) {
Expand Down
3 changes: 3 additions & 0 deletions graph/src/data/subgraph/api_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions graph/src/env/mappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>,
Expand Down
9 changes: 9 additions & 0 deletions graph/src/schema/input_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,15 @@ impl InputSchema {
.map(|fields| fields.contains(&field))
.unwrap_or(false)
}

pub fn has_field_with_name(&self, entity_type: &EntityType, field: &str) -> bool {
let field = self.inner.pool.lookup(field);

match field {
Some(field_atom) => self.has_field(entity_type, field_atom),
None => false,
}
}
}

/// Create a new pool that contains the names of all the types defined
Expand Down
50 changes: 41 additions & 9 deletions runtime/test/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,8 +1225,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());
Expand Down Expand Up @@ -1324,7 +1329,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");
Expand Down Expand Up @@ -1414,7 +1419,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();
Expand All @@ -1437,8 +1448,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(
Expand All @@ -1449,8 +1459,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);
}
61 changes: 60 additions & 1 deletion runtime/wasm/src/host_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 never::Never;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -199,9 +248,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");
Expand Down
15 changes: 15 additions & 0 deletions tests/runner-tests/api-version/abis/Contract.abi
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[
{
"anonymous": false,
"inputs": [
{
"indexed": false,
"internalType": "string",
"name": "testCommand",
"type": "string"
}
],
"name": "TestEvent",
"type": "event"
}
]
3 changes: 3 additions & 0 deletions tests/runner-tests/api-version/data.0.0.7.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"apiVersion": "0.0.7"
}
3 changes: 3 additions & 0 deletions tests/runner-tests/api-version/data.0.0.8.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"apiVersion": "0.0.8"
}
29 changes: 29 additions & 0 deletions tests/runner-tests/api-version/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
4 changes: 4 additions & 0 deletions tests/runner-tests/api-version/schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type TestResult @entity {
id: ID!
message: String!
}
15 changes: 15 additions & 0 deletions tests/runner-tests/api-version/src/mapping.ts
Original file line number Diff line number Diff line change
@@ -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();
}
Loading

0 comments on commit e12c79c

Please sign in to comment.