0xMiden / 0xMiden/protocol

PartialStorage::read_from skips the map-root check that PartialStorage::new enforces

未关闭
#3,533 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
Rust
星标
132
派生
167
平均合并
1 天 23 小时
30 天内合并 PR
110

描述

Was looking through the account storage code and noticed something odd in `crates/miden-protocol/src/account/storage/partial.rs`.

`PartialStorage::new` checks that every `PartialStorageMap` passed in has a root matching one of the slot roots in the storage header, and returns `StorageMapRootNotFound` if not (this check itself was added/fixed in #1647). But `PartialStorage`'s `Deserializable::read_from` doesn't go through `new` at all, it just builds the struct straight from the deserialized fields:

```rust
impl Deserializable for PartialStorage {
fn read_from(source: &mut R) -> Result {
let header: AccountStorageHeader = source.read()?;
let map_smts: BTreeMap = source.read()?;
let commitment = header.to_commitment();
Ok(PartialStorage { header, maps: map_smts, commitment })
}
}
```

So if you deserialize bytes where a map's root doesn't line up with any slot root in the header, it just goes through, no error, even though `new()` would reject exactly this case. Same class of issue as #3494 (BlockSignatures): there's a validating constructor, but the Deserializable impl doesn't route through it, and in this case the root-check in `new()` was specifically added by #1647 without the deserialize path ever being updated to match.

I confirmed this locally with a test: constructed a `PartialStorage` bypassing `new()` (same shape `read_from` produces), round-tripped it through `to_bytes()` / `read_from_bytes()`, and it deserializes successfully with a map whose root isn't present in the header at all. `new()` correctly rejects the same data.

Happy to send a PR for this if it can be assigned to me — I already have a working regression test for it, the fix itself is just having `read_from` validate the roots the same way `new()` does.

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。