CosmWasm / CosmWasm/cw-storage-plus

Bug with SnapshotMap remove when value is `()`

Open
#97 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
51
Forks
34
PR merge metrics
No merged PRs in 30d

Description

When value removing `()` value, somehow the value is not stored in the changelog. so when calling `may_load_at_height`, the old value returned is `None` when it should be `Some(())`. Example below, illustrate it better.

This code below illustrate the problem.
```rust
type Firstname = String;
const PEOPLE: SnapshotMap = SnapshotMap::new(
"people",
"people_checkpoint",
"people_changelog",
Strategy::EveryBlock,
);

let mut deps = mock_dependencies();
let mut env = mock_env();

let firstname = "john".to_string();

PEOPLE
.save(&mut deps.storage, firstname.clone(), &(), env.block.height)
.unwrap();

// assert that john exists
let res = PEOPLE.may_load(&deps.storage, firstname.clone()).unwrap();
assert_eq!(res, Some(()));

let res = PEOPLE
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height + 1)
.unwrap();
assert_eq!(res, Some(()));

// block advances
env.block.height += 10;

// remove john
PEOPLE
.remove(&mut deps.storage, firstname.clone(), env.block.height)
.unwrap();

// assert that john does not exist
let res = PEOPLE.may_load(&deps.storage, firstname.clone()).unwrap();
assert_eq!(res, None);
let res = PEOPLE
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height + 1)
.unwrap();
assert_eq!(res, None);

// assert that john used to exists in previous block height
let res = PEOPLE
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height - 1)
.unwrap();
assert_eq!(res, Some(())); // ❌ ERROR: res is None
```

When the value stored is change from `()` to `bool`, it is able to retrieve correctly.
```rust
type Firstname = String;
const PEOPLE2: SnapshotMap = SnapshotMap::new(
"people",
"people_checkpoint",
"people_changelog",
Strategy::EveryBlock,
);

let mut deps = mock_dependencies();
let mut env = mock_env();

let firstname = "john".to_string();

PEOPLE2
.save(
&mut deps.storage,
firstname.clone(),
&true,
env.block.height,
)
.unwrap();

// assert that john exists
let res = PEOPLE2.may_load(&deps.storage, firstname.clone()).unwrap();
assert_eq!(res, Some(true));

let res = PEOPLE2
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height + 1)
.unwrap();
assert_eq!(res, Some(true));

// block advances
env.block.height += 10;

// remove john
PEOPLE2
.remove(&mut deps.storage, firstname.clone(), env.block.height)
.unwrap();

// assert that john does not exist
let res = PEOPLE2.may_load(&deps.storage, firstname.clone()).unwrap();
assert_eq!(res, None);
let res = PEOPLE2
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height + 1)
.unwrap();
assert_eq!(res, None);

// assert that john used to exists in previous block height
let res = PEOPLE2
.may_load_at_height(&deps.storage, firstname.clone(), env.block.height - 1)
.unwrap();
assert_eq!(res, Some(true)); // ✅Passes
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Use the SnapshotMap reproduction in the issue as the starting test, focusing on remove and may_load_at_height with value (). Compare its behavior with the bool case. Done when historical lookup returns Some(()) after removal, current lookup remains None, and the regression is covered by a test.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.