Skip to content
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

- [#6913](https://github.com/ChainSafe/forest/issues/6913): Added support for NV28 for devnets. Use `FOREST_FIREHORSE_HEIGHT` environment variable to set the upgrade height.

- [#6926](https://github.com/ChainSafe/forest/pull/6926): Added strict JSON validation to deny unknown fields in RPC request parameters and response results when `FOREST_STRICT_JSON` is enabled.
Comment thread
sudo-shashank marked this conversation as resolved.

### Changed

- [#6939](https://github.com/ChainSafe/forest/pull/6939): Refactored snapshot export and garbage collection logic to use disk-backed hash set for de-de-duplicating reachable blocks. This results in less RAM usage (~6-7GiB) and more disk usage (~7-8GiB on mainnet).
Expand Down
11 changes: 11 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ schemars = { version = "1", features = ["chrono04", "uuid1"] }
scopeguard = "1"
semver = "1"
serde = { workspace = true }
serde_ignored = "0.1"
serde_ipld_dagcbor = "0.6"
serde_json = { version = "1", features = ["raw_value"] }
serde_with = { version = "3", features = ["chrono_0_4"] }
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/users/reference/env_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ process.
| `FOREST_JWT_DISABLE_EXP_VALIDATION` | 1 or true | empty | 1 | Whether or not to disable JWT expiration validation |
| `FOREST_ETH_BLOCK_CACHE_SIZE` | positive integer | 500 | 1 | The size of Eth block cache |
| `FOREST_RPC_BACKFILL_FULL_TIPSET_FROM_NETWORK` | 1 or true | false | 1 | Whether or not to backfill full tipsets from the p2p network |
| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests |
| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys and reject unknown fields in RPC requests and responses |
| `FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH` | URL or file path | empty | `/var/tmp/forest_snapshot_calibnet.forest.car.zst` | Override snapshot path for `--auto-download-snapshot` |
| `FOREST_DOWNLOAD_CONNECTIONS` | positive integer | 5 | 10 | Number of parallel HTTP connections for downloading snapshots |
| `FOREST_ETH_V1_DISABLE_F3_FINALITY_RESOLUTION` | 1 or true | empty | 1 | Whether or not to disable F3 finality resolution in Eth `v1` RPC methods |
Expand Down
3 changes: 3 additions & 0 deletions scripts/tests/api_compare/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ services:
- RUST_LOG=info,forest::tool::subcommands=debug
- FOREST_RPC_DEFAULT_TIMEOUT=120
- FIL_PROOFS_PARAMETER_CACHE=${FIL_PROOFS_PARAMETER_CACHE}
- FOREST_STRICT_JSON=1
entrypoint: ["/bin/bash", "-c"]
user: 0:0
command:
Expand Down Expand Up @@ -310,6 +311,7 @@ services:
- RUST_LOG=info,forest::tool::subcommands=debug
- FOREST_RPC_DEFAULT_TIMEOUT=120
- FIL_PROOFS_PARAMETER_CACHE=${FIL_PROOFS_PARAMETER_CACHE}
- FOREST_STRICT_JSON=1
entrypoint: ["/bin/bash", "-c"]
user: 0:0
command:
Expand Down Expand Up @@ -379,6 +381,7 @@ services:
- RUST_LOG=info,forest::tool::subcommands=debug
- FOREST_RPC_DEFAULT_TIMEOUT=120
- FIL_PROOFS_PARAMETER_CACHE=${FIL_PROOFS_PARAMETER_CACHE}
- FOREST_STRICT_JSON=1
entrypoint: ["/bin/bash", "-c"]
user: 0:0
command:
Expand Down
17 changes: 16 additions & 1 deletion src/lotus_json/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::*;

use crate::shim::{address::Address, econ::TokenAmount, message::Message};
use fvm_ipld_encoding::RawBytes;
use ::cid::Cid;

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
Expand Down Expand Up @@ -40,13 +41,23 @@ pub struct MessageLotusJson {
default
)]
params: Option<RawBytes>,
#[schemars(with = "LotusJson<Option<Cid>>")]
#[serde(
with = "crate::lotus_json",
rename = "CID",
default,
skip_serializing_if = "Option::is_none"
)]
cid: Option<Cid>,
}

impl HasLotusJson for Message {
type LotusJson = MessageLotusJson;

#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
let msg = Message::default();
let cid = msg.cid();
vec![(
json!({
"From": "f00",
Expand All @@ -59,12 +70,14 @@ impl HasLotusJson for Message {
"To": "f00",
"Value": "0",
"Version": 0,
"CID": { "/": cid.to_string() },
}),
Message::default(),
msg,
)]
}

fn into_lotus_json(self) -> Self::LotusJson {
let cid = self.cid();
let Self {
version,
from,
Expand All @@ -88,6 +101,7 @@ impl HasLotusJson for Message {
gas_premium,
method: method_num,
params: Some(params),
cid: Some(cid),
}
}

Expand All @@ -103,6 +117,7 @@ impl HasLotusJson for Message {
gas_premium,
method,
params,
cid: _,
} = lotus_json;
Self {
version,
Expand Down
5 changes: 4 additions & 1 deletion src/lotus_json/signed_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ impl HasLotusJson for SignedMessage {

#[cfg(test)]
fn snapshots() -> Vec<(serde_json::Value, Self)> {
let msg = Message::default();
let msg_cid = msg.cid();
vec![(
json!({
"Message": {
Expand All @@ -52,14 +54,15 @@ impl HasLotusJson for SignedMessage {
"To": "f00",
"Value": "0",
"Version": 0,
"CID": { "/": msg_cid.to_string() },
},
"Signature": {"Type": 2, "Data": "aGVsbG8gd29ybGQh"},
"CID": {
"/": "bafy2bzaced3xdk2uf6azekyxgcttujvy3fzyeqmibtpjf2fxcpfdx2zcx4s3g"
},
}),
SignedMessage {
message: Message::default(),
message: msg,
signature: Signature {
sig_type: crate::shim::crypto::SignatureType::Bls,
bytes: Vec::from_iter(*b"hello world!"),
Expand Down
86 changes: 83 additions & 3 deletions src/rpc/json_validator.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
// Copyright 2019-2026 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

//! JSON validation utilities for detecting duplicate keys before serde_json processing.
//! JSON validation utilities for RPC requests and responses processing.
//!
//! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy
//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behavior in RPC calls.
//! - **Duplicate key detection**: `serde_json` automatically deduplicates keys at parse time
//! using a "last-write-wins" strategy. This means JSON like `{"/":"cid1", "/":"cid2"}` will
//! keep only the last value, which can lead to unexpected behavior in RPC calls.
//! - **Unknown field detection**: `serde_json` silently ignores unknown fields by default.
//! In strict mode, [`from_value_rejecting_unknown_fields`] applies to RPC request and
//! responses.
//!
//! All of this is gated behind the `FOREST_STRICT_JSON` environment variable.

use ahash::HashSet;
use serde::de::DeserializeOwned;

pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON";

Expand Down Expand Up @@ -50,9 +57,32 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> {
check_value(&value)
}

/// De-serializes a [`serde_json::Value`] into `T`, rejecting unknown fields when strict mode is
/// enabled. When strict mode is off, this is equivalent to [`serde_json::from_value`].
pub fn from_value_rejecting_unknown_fields<T: DeserializeOwned>(
value: serde_json::Value,
) -> Result<T, serde_json::Error> {
if !is_strict_mode() {
return serde_json::from_value(value);
}
let mut unknown = Vec::new();
let result: T = serde_ignored::deserialize(value, |path| {
unknown.push(path.to_string());
})?;
if !unknown.is_empty() {
return Err(serde::de::Error::custom(format!(
"unknown field(s): {}. Set {STRICT_JSON_ENV}=0 to disable this check",
unknown.join(", ")
)));
}
Ok(result)
}

#[cfg(test)]
mod tests {
use super::*;
use serde::Deserialize;
use serde_json::json;
use serial_test::serial;

fn with_strict_mode<F>(enabled: bool, f: F)
Expand Down Expand Up @@ -131,4 +161,54 @@ mod tests {
assert!(result.unwrap_err().contains("duplicate key '/'"));
});
}

#[derive(Debug, Deserialize, PartialEq)]
struct RpcTestReq {
name: String,
value: i32,
}

#[test]
#[serial]
fn test_unknown_fields_known_only() {
with_strict_mode(true, || {
let val = json!({"name": "alice", "value": 42});
let result = from_value_rejecting_unknown_fields::<RpcTestReq>(val);
assert_eq!(
result.unwrap(),
RpcTestReq {
name: "alice".into(),
value: 42
}
);
});
}

#[test]
#[serial]
fn test_unknown_fields_detected() {
with_strict_mode(true, || {
let val = json!({"name": "alice", "value": 42, "extra": true});
let err = from_value_rejecting_unknown_fields::<RpcTestReq>(val)
.expect_err("expected Err when unknown JSON field is present under strict mode");
let msg = err.to_string();
assert!(
msg.contains("unknown field(s)") && msg.contains("extra"),
"got: {msg}"
);
});
}

#[test]
#[serial]
fn test_unknown_fields_strict_mode_off() {
with_strict_mode(false, || {
let val = json!({"name": "alice", "value": 42, "extra": true});
let result = from_value_rejecting_unknown_fields::<RpcTestReq>(val);
assert!(
result.is_ok(),
"unknown fields should be allowed when strict mode is off"
);
});
}
}
21 changes: 3 additions & 18 deletions src/rpc/methods/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl RpcMethod<1> for ChainGetMessage {
const DESCRIPTION: Option<&'static str> = Some("Returns the message with the specified CID.");

type Params = (Cid,);
type Ok = FlattenedApiMessage;
type Ok = Message;

async fn handle(
ctx: Ctx<impl Blockstore>,
Expand All @@ -193,13 +193,10 @@ impl RpcMethod<1> for ChainGetMessage {
.store()
.get_cbor(&message_cid)?
.with_context(|| format!("can't find message with cid {message_cid}"))?;
let message = match chain_message {
Ok(match chain_message {
ChainMessage::Signed(m) => Arc::unwrap_or_clone(m).into_message(),
ChainMessage::Unsigned(m) => Arc::unwrap_or_clone(m),
};

let cid = message.cid();
Ok(FlattenedApiMessage { message, cid })
})
}
}

Expand Down Expand Up @@ -1511,18 +1508,6 @@ pub struct ApiMessage {

lotus_json_with_self!(ApiMessage);

#[derive(Serialize, Deserialize, JsonSchema, Clone, Debug, Eq, PartialEq)]
pub struct FlattenedApiMessage {
#[serde(flatten, with = "crate::lotus_json")]
#[schemars(with = "LotusJson<Message>")]
pub message: Message,
#[serde(rename = "CID", with = "crate::lotus_json")]
#[schemars(with = "LotusJson<Cid>")]
pub cid: Cid,
}

lotus_json_with_self!(FlattenedApiMessage);

#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
pub struct ForestChainExportParams {
pub version: FilecoinSnapshotVersion,
Expand Down
3 changes: 3 additions & 0 deletions src/rpc/methods/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ impl RpcMethod<0> for Version {
// For the API v0, we don't support it but it should be `1.5.0`.
api_version: ShiftingVersion::new(2, 3, 0),
block_delay: ctx.chain_config().block_delay_secs,
agent: None,
})
}
}
Expand Down Expand Up @@ -106,6 +107,8 @@ pub struct PublicVersion {
#[serde(rename = "APIVersion")]
pub api_version: ShiftingVersion,
pub block_delay: u32,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub agent: Option<String>,
}
lotus_json_with_self!(PublicVersion);

Expand Down
2 changes: 2 additions & 0 deletions src/rpc/methods/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4492,6 +4492,8 @@ mod test {
invoked_actor: None,
gas_charges: vec![],
subcalls: vec![],
logs: vec![],
ipld_ops: vec![],
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/rpc/methods/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use super::state::InvocResult;
use crate::blocks::Tipset;
use crate::chain::{BASE_FEE_MAX_CHANGE_DENOM, BLOCK_GAS_TARGET};
use crate::message::{ChainMessage, MessageRead as _, MessageReadWrite as _, SignedMessage};
use crate::rpc::chain::FlattenedApiMessage;
use crate::rpc::{ApiPaths, Ctx, Permission, RpcMethod, error::ServerError, types::*};
use crate::shim::executor::ApplyRet;
use crate::shim::{
Expand Down Expand Up @@ -305,16 +304,14 @@ impl RpcMethod<3> for GasEstimateMessageGas {
Some("Returns the estimated gas for the given parameters.");

type Params = (Message, Option<MessageSendSpec>, ApiTipsetKey);
type Ok = FlattenedApiMessage;
type Ok = Message;

async fn handle(
ctx: Ctx<impl Blockstore + Send + Sync + 'static>,
(msg, spec, tsk): Self::Params,
_: &http::Extensions,
) -> Result<Self::Ok, ServerError> {
let message = estimate_message_gas(&ctx, msg, spec, tsk).await?;
let cid = message.cid();
Ok(FlattenedApiMessage { message, cid })
estimate_message_gas(&ctx, msg, spec, tsk).await
}
}

Expand Down
Loading
Loading