sway icon indicating copy to clipboard operation
sway copied to clipboard
verified publisher icon FuelLabs

Forbid storage key collisions between `storage` and `StorageMap`

Open ironcev opened this issue 1 year ago • 5 comments

Description

This PR fixes #6317 by introducing two "storage domains", one for the storage fields defined inside of the storage declaration, and a different one for storage slots generated inside of the StorageMap.

The PR strictly fixes the issue described in #6317 by applying the recommendation proposed in the issue. A general approach to storage key domains will be discussed as a part of the Configurable and composable storage RFC.

Additionally, the PR:

  • adds expressive diagnostics for the duplicated storage keys warning.
  • removes the misleading internal compiler error that always followed the storage key type mismatch error (see demo below).

Closes #6317.

Breaking Changes

The PR changes the way how storage keys are generated for storage fields. Instead of sha256("storage::ns1::ns2.field_name") we now use sha256((0u8, "storage::ns1::ns2.field_name")).

This is a breaking change for those who relied on the storage key calculation.

Demo

Before, every type-mismatch was always followed by an ICE, because the compilation continued despite the type-mismatch.

Mismatched types and ICE - Before

This is solved now, and the type-mismatch has a dedicated help message.

Mismatched types and ICE - After

Checklist

  • [x] I have linked to any relevant issues.
  • [x] I have commented my code, particularly in hard-to-understand areas.
  • [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • [x] I have added tests that prove my fix is effective or that my feature works.
  • [x] I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • [x] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • [x] I have requested a review from the relevant team or maintainers.

ironcev avatar Aug 25 '24 19:08 ironcev

Benchmark for b830f90

Click to view benchmark
Test Base PR %
code_action 5.0±0.01ms 5.1±0.09ms +2.00%
code_lens 283.6±6.98ns 285.6±7.94ns +0.71%
compile 2.6±0.04s 2.7±0.05s +3.85%
completion 4.6±0.05ms 4.5±0.01ms -2.17%
did_change_with_caching 2.6±0.03s 2.5±0.04s -3.85%
document_symbol 916.9±35.99µs 903.3±29.60µs -1.48%
format 70.4±0.65ms 67.6±1.08ms -3.98%
goto_definition 338.8±5.42µs 345.8±7.19µs +2.07%
highlight 8.7±0.01ms 8.7±0.05ms 0.00%
hover 489.7±4.49µs 498.6±5.54µs +1.82%
idents_at_position 118.0±0.29µs 117.4±0.39µs -0.51%
inlay_hints 622.9±22.41µs 628.0±21.49µs +0.82%
on_enter 2.1±0.05µs 2.0±0.08µs -4.76%
parent_decl_at_position 3.6±0.04ms 3.6±0.04ms 0.00%
prepare_rename 337.1±5.52µs 344.4±8.52µs +2.17%
rename 9.0±0.01ms 9.0±0.14ms 0.00%
semantic_tokens 1244.4±11.46µs 1246.6±8.35µs +0.18%
token_at_position 336.3±2.48µs 335.2±1.45µs -0.33%
tokens_at_position 3.6±0.03ms 3.5±0.01ms -2.78%
tokens_for_file 400.4±2.18µs 404.4±3.37µs +1.00%
traverse 34.1±0.89ms 34.6±0.54ms +1.47%

github-actions[bot] avatar Aug 25 '24 19:08 github-actions[bot]

Benchmark for 4944773

Click to view benchmark
Test Base PR %
code_action 5.1±0.10ms 5.0±0.10ms -1.96%
code_lens 290.9±9.27ns 288.6±9.83ns -0.79%
compile 2.7±0.03s 2.7±0.07s 0.00%
completion 4.5±0.06ms 4.5±0.03ms 0.00%
did_change_with_caching 2.5±0.06s 2.6±0.03s +4.00%
document_symbol 894.6±32.14µs 960.8±15.76µs +7.40%
format 72.3±1.04ms 67.7±0.53ms -6.36%
goto_definition 341.6±7.17µs 341.3±7.31µs -0.09%
highlight 8.7±0.17ms 8.8±0.16ms +1.15%
hover 493.7±6.73µs 496.2±9.74µs +0.51%
idents_at_position 118.6±0.41µs 118.5±0.71µs -0.08%
inlay_hints 631.3±12.00µs 631.4±28.47µs +0.02%
on_enter 2.1±0.05µs 2.0±0.07µs -4.76%
parent_decl_at_position 3.6±0.03ms 3.6±0.03ms 0.00%
prepare_rename 342.0±4.27µs 341.5±7.01µs -0.15%
rename 9.0±0.16ms 9.0±0.22ms 0.00%
semantic_tokens 1228.1±13.18µs 1252.6±19.42µs +1.99%
token_at_position 338.6±3.70µs 334.1±2.17µs -1.33%
tokens_at_position 3.6±0.06ms 3.6±0.02ms 0.00%
tokens_for_file 398.8±2.36µs 403.3±3.21µs +1.13%
traverse 33.6±0.66ms 33.8±0.87ms +0.60%

github-actions[bot] avatar Aug 26 '24 09:08 github-actions[bot]

Benchmark for f370432

Click to view benchmark
Test Base PR %
code_action 5.2±0.17ms 5.4±0.19ms +3.85%
code_lens 284.3±11.69ns 285.3±12.64ns +0.35%
compile 2.8±0.05s 2.8±0.09s 0.00%
completion 4.8±0.19ms 4.8±0.15ms 0.00%
did_change_with_caching 2.7±0.04s 2.8±0.04s +3.70%
document_symbol 981.3±30.14µs 883.9±23.15µs -9.93%
format 71.3±0.89ms 69.2±0.91ms -2.95%
goto_definition 338.6±6.34µs 338.0±7.68µs -0.18%
highlight 9.0±0.22ms 9.4±0.30ms +4.44%
hover 545.9±5.93µs 491.7±6.54µs -9.93%
idents_at_position 118.0±0.48µs 117.3±0.29µs -0.59%
inlay_hints 629.7±27.39µs 641.6±24.28µs +1.89%
on_enter 2.0±0.04µs 1997.1±84.59ns -0.14%
parent_decl_at_position 3.6±0.07ms 3.8±0.08ms +5.56%
prepare_rename 335.5±5.47µs 333.1±6.80µs -0.72%
rename 9.4±0.21ms 9.5±0.25ms +1.06%
semantic_tokens 1194.1±19.04µs 1209.0±10.56µs +1.25%
token_at_position 330.1±1.51µs 329.3±2.38µs -0.24%
tokens_at_position 3.6±0.08ms 3.8±0.10ms +5.56%
tokens_for_file 396.4±2.61µs 397.4±3.10µs +0.25%
traverse 36.0±0.93ms 36.2±0.83ms +0.56%

github-actions[bot] avatar Aug 26 '24 13:08 github-actions[bot]

As discussed, converting to draft until we get some initial infrastructure for feature flags as discussed in the RFC on bringing changes to the compiler

ironcev avatar Aug 27 '24 09:08 ironcev

CodSpeed Performance Report

Merging #6466 will not alter performance

Comparing ironcev/6317-storage-collision (02ca4be) with master (6e31144)

Summary

✅ 22 untouched benchmarks

codspeed-hq[bot] avatar Aug 29 '24 23:08 codspeed-hq[bot]