Repository: paritytech/ink
Pull request #425 (2020-05-25 18:47:27)

Fix unreachable bug in storage2::Stash and add regression test

codecov-commenter: # [Codecov](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=h1) Report
> Merging [#425](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=desc) into [master](https://codecov.io/gh/paritytech/ink/commit/b302ebcfe15005fe36a20dc279cbc47e0782e26f&el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/paritytech/ink/pull/425/graphs/tree.svg?width=650&height=150&src=pr&token=tYyVaCZb7o)](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   86.07%   86.16%   +0.08%     
==========================================
  Files         135      135              
  Lines        5854     5855       +1     
==========================================
+ Hits         5039     5045       +6     
+ Misses        815      810       -5     
```


| [Impacted Files](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [core/src/storage2/collections/stash/tests.rs](https://codecov.io/gh/paritytech/ink/pull/425/diff?src=pr&el=tree#diff-Y29yZS9zcmMvc3RvcmFnZTIvY29sbGVjdGlvbnMvc3Rhc2gvdGVzdHMucnM=) | `100.00% <ø> (ø)` | |
| [core/src/storage2/collections/stash/mod.rs](https://codecov.io/gh/paritytech/ink/pull/425/diff?src=pr&el=tree#diff-Y29yZS9zcmMvc3RvcmFnZTIvY29sbGVjdGlvbnMvc3Rhc2gvbW9kLnJz) | `95.80% <100.00%> (+3.55%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=footer). Last update [b302ebc...8d54b4d](https://codecov.io/gh/paritytech/ink/pull/425?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/core/src/storage2/collections/stash/mod.rs b/core/src/storage2/collections/stash/mod.rs
index 883ec008d8..288a58734d 100644
--- a/core/src/storage2/collections/stash/mod.rs
+++ b/core/src/storage2/collections/stash/mod.rs
@@ -304,6 +304,7 @@ where
         if prev_vacant == removed_index && next_vacant == removed_index {
             // There is no other vacant entry left in the storage stash so
             // there is nothing to update. Bail out early.
+            self.header.last_vacant = self.header.len;
             return
         }
         if prev_vacant == next_vacant {


Test patch:
diff --git a/core/src/storage2/collections/stash/tests.rs b/core/src/storage2/collections/stash/tests.rs
index bae3cc53f7..cfd500cd79 100644
--- a/core/src/storage2/collections/stash/tests.rs
+++ b/core/src/storage2/collections/stash/tests.rs
@@ -22,6 +22,25 @@ use crate::{
 };
 use ink_primitives::Key;
 
+#[test]
+fn regression_stash_unreachable_minified() {
+    // This regression has been discovered in the ERC721 example implementation
+    // `approved_for_all_works` unit test. The fix was to adjust
+    // `Stash::remove_vacant_entry` to update `header.last_vacant` if the
+    // removed index was the last remaining vacant index in the stash.
+    env::test::run_test::<env::DefaultEnvTypes, _>(|_| {
+        let mut stash: StorageStash<u32> = StorageStash::new();
+        stash.put(1);
+        stash.put(2);
+        stash.take(0);
+        stash.put(99);
+        stash.take(1);
+        stash.put(99);
+        Ok(())
+    })
+    .unwrap()
+}
+
 #[test]
 fn new_works() {
     // `StorageVec::new`