Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage slot 0 is unusable #1692

Closed
LHerskind opened this issue Aug 21, 2023 · 3 comments · Fixed by #1884 or #1900
Closed

Storage slot 0 is unusable #1692

LHerskind opened this issue Aug 21, 2023 · 3 comments · Fixed by #1884 or #1900
Assignees
Labels
T-bug Type: Bug. Something is broken.

Comments

@LHerskind
Copy link
Contributor

Using storage slot 0 in your noir contract will make the storage non-updatable, e.g., writes wont take effect, it will just be a returning value 0 every time it is read afterwards.

This should either be allowed, or it should throw an error at compile time to make sure the contracts don't to this.

@LHerskind LHerskind added the T-bug Type: Bug. Something is broken. label Aug 21, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Aug 21, 2023
@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented Aug 21, 2023

We probably can't catch it at compile time, yet, but we could catch it at runtime within each state variable constructor (or within a helper function which is called by each state variable constructor).

Tagging @sirasistant @LeilaWang , as there's some work being done to give nice runtime errors. This might be useful for this issue as well assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");

It's a shame Noir can't do compile-time assertions, like barretenberg can :P

@sirasistant
Copy link
Collaborator

If there is an assertion with all the values constant, I think it will be caught at compile time! So we can introduce this slot assertion in our libs

@sirasistant
Copy link
Collaborator

And the devex will be even better with assert messages

@iAmMichaelConnor iAmMichaelConnor added this to the 📢 Initial Public Sandbox Release milestone Aug 25, 2023
@benesjan benesjan self-assigned this Aug 30, 2023
@benesjan benesjan moved this from Todo to In Progress in A3 Aug 30, 2023
@benesjan benesjan moved this from In Progress to In Review in A3 Aug 30, 2023
iAmMichaelConnor pushed a commit that referenced this issue Aug 31, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 Aug 31, 2023
PhilWindle pushed a commit that referenced this issue Sep 5, 2023
🤖 I have created a new Aztec Packages release
---


##
[0.1.0-alpha50](v0.1.0-alpha49...v0.1.0-alpha50)
(2023-09-05)


### ⚠ BREAKING CHANGES

* update to acvm 0.24.0
([#1925](#1925))

### Features

* **892:** add hints for matching transient read requests with
correspondi…
([#1995](#1995))
([0955bb7](0955bb7))
* Add support for assert messages & runtime call stacks
([#1997](#1997))
([ac68837](ac68837))
* **Aztec.nr:** Kernel return types abstraction
([#1924](#1924))
([3a8e702](3a8e702))
* **ci:** use content hash in build system, restrict docs build to *.ts
or *.cpp
([#1953](#1953))
([0036e07](0036e07))
* do not allow slot 0 in `noir-libs`
([#1884](#1884))
([54094b4](54094b4)),
closes
[#1692](#1692)
* throwing when submitting a duplicate tx of a settled one
([#1880](#1880))
([9ad768f](9ad768f)),
closes
[#1810](#1810)
* typos, using Tx.clone functionality, better naming
([#1976](#1976))
([00bca67](00bca67))


### Bug Fixes

* add retry_10 around ensure_repo
([#1963](#1963))
([0afde39](0afde39))
* Adds Mac cross compile flags into barretenberg
([#1954](#1954))
([3aaf91e](3aaf91e))
* bb meta-data
([#1960](#1960))
([712e0a0](712e0a0))
* **bb.js:** (breaking change) bundles bb.js properly so that it works
in the browser and in node
([#1855](#1855))
([1aa6f59](1aa6f59))
* Benchmark preset uses clang16
([#1902](#1902))
([4f7eeea](4f7eeea))
* build
([#1906](#1906))
([8223be1](8223be1))
* **ci:** Incorrect content hash in some build targets
([#1973](#1973))
([0a2a515](0a2a515))
* circuits should not link openmp with -DMULTITHREADING
([#1929](#1929))
([cd1a685](cd1a685))
* compilation on homebrew clang 16.06
([#1937](#1937))
([c611582](c611582))
* docs preprocessor line numbers and errors
([#1883](#1883))
([4e7e290](4e7e290))
* ensure CLI command doesn't fail due to missing client version
([#1895](#1895))
([88086e4](88086e4))
* error handling in acir simulator
([#1907](#1907))
([165008e](165008e))
* Fix off by one in circuits.js when fetching points from transcript
([#1993](#1993))
([cec901f](cec901f))
* format.sh issues
([#1946](#1946))
([f24814b](f24814b))
* master
([#1981](#1981))
([6bfb053](6bfb053))
* More accurate c++ build pattern
([#1962](#1962))
([21c2f8e](21c2f8e))
* polyfill by bundling fileURLToPath
([#1949](#1949))
([1b2de01](1b2de01))
* Set correct version of RPC & Sandbox when deploying tagged commit
([#1914](#1914))
([898c50d](898c50d))
* typescript lookup of aztec.js types
([#1948](#1948))
([22901ae](22901ae))
* unify base64 interface between mac and linux (cherry-picked)
([#1968](#1968))
([ee24b52](ee24b52))
* Update docs search config
([#1920](#1920))
([c8764e6](c8764e6))
* update docs search keys
([#1931](#1931))
([03b200c](03b200c))


### Miscellaneous

* **1407:** remove forwarding witnesses
([#1930](#1930))
([cc8bc8f](cc8bc8f)),
closes
[#1407](#1407)
* **1879:** add use of PrivateKernelPublicInputs in TS whenever relevant
([#1911](#1911))
([8d5f548](8d5f548))
* acir tests are no longer base64 encoded
([#1854](#1854))
([7fffd16](7fffd16))
* Add back double verify proof to test suite
([#1986](#1986))
([f8688d7](f8688d7))
* add CLI test to canary flow
([#1918](#1918))
([cc68958](cc68958)),
closes
[#1903](#1903)
* Add safemath noir testing
([#1967](#1967))
([cb1f1ec](cb1f1ec))
* **Aztec.nr:** remove implicit imports
([#1901](#1901))
([c7d5190](c7d5190))
* **Aztec.nr:** Remove the open keyword from public functions
([#1917](#1917))
([4db8603](4db8603))
* **ci:** build docs on every pr
([#1955](#1955))
([c200bc5](c200bc5))
* Enable project-specific releases for dockerhub too
([#1721](#1721))
([5d2c082](5d2c082))
* reduce max circuit size in bb binary
([#1942](#1942))
([c61439b](c61439b))
* Reference noir master for acir tests
([#1969](#1969))
([86b72e1](86b72e1))
* remove debug output from `run_acir_tests` script
([#1970](#1970))
([74c83c5](74c83c5))
* storing `&mut context` in state vars
([#1926](#1926))
([89a7a3f](89a7a3f)),
closes
[#1805](#1805)
* sync bb master
([#1947](#1947))
([eed58e1](eed58e1))
* update to acvm 0.24.0
([#1925](#1925))
([e728304](e728304))
* Update to acvm 0.24.1
([#1978](#1978))
([31c0a02](31c0a02))
* updating docs to clang16
([#1875](#1875))
([a248dae](a248dae))


### Documentation

* **keys:** Complete addresses are now broadcast
([#1975](#1975))
([92068ad](92068ad)),
closes
[#1936](#1936)
* limitations, privacy, roadmap
([#1759](#1759))
([0cdb27a](0cdb27a))
* put dev docs before spec
([#1944](#1944))
([f1b29cd](f1b29cd))
* storage and state variables
([#1725](#1725))
([fc72f84](fc72f84))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
codygunton pushed a commit that referenced this issue Jan 23, 2024
* Attempt to fix production deployment (#1683)

* Add stefan to mainframe. Deployment publishes bb.js and blockchain to npm.

* Fix block scanning (#1692)

* Attempt to fix deploy.

* Fix dai-permit issues. Providers did not offset `v` properly (#1698)

* fix: perform offset to `sig.v if 0 or 1 for `signTypedData`

* fix: remove git-submodules

* Force to -O2 with binaryen installed to prevent aes bug.

* Updated fork block number (#1685)

* Updated fork block number

* Updated fork block and element test tranche

* Removed no longer used env variable

* Updated Faucet index page amounts

* [zk-money] Jcf/aave (#1642)

* Configure aave defi cards

* copy-paste error

* Increase test timeout

Co-authored-by: PhilWindle <philip.windle@gmail.com>

* fix aave token addresses (#1701)

* Fuzzer-found bugs that are not related to bigfield (#1605)

* Added a regression test to detect original parallelized asm bugs

* FIxed parallelized SQR asm bugs

* Added comment that Emerson wanted

* Fix for create_range_constraint in the constant case

* Various safe_uint bug fixes having to do with not handling constant cases

* Fixed the comparison issue for the point at infinity

* Added regression tests and fixed 3 more missing flags in optimized SQR

* Fixed tabulation in asm_macros.hpp

* Fixed Montgomery Issue that Adrian Found

* Fixed issue with negative zero found by Guido

* FIxed same self_neg bug found by Guido

* Renamed 1 test

* Fixed one more SQR bug

* Added extra regression tests

* Slightly moved one test

* Added TODO comments into field_impl for potential optimisations

* Addressed Zac's comments

Co-authored-by: zac-williamson <blorktronics@gmail.com>

* Fixing errors in addition and subtraction of field elements with moduli > 254 bits (#1702)

* Added test

* Added bugfixes

* Added comments

* Use 400k gas bridge for aave (#1705)

* Add versioning to falafel status endpoint. Refresh zk-money on version mismatch. (#1674)

* all get/post reqs from sdk to falafel now include  header

* falafel expects all endpoint requests to be given a 'version' header. SDK now gives all reqs a version. If falafel sees a req with version that does not match falafel version, it responds with error 402. ServerRollupProvider now emits versionMismatch whenever one occurs. zk-money promotes this event and forces refresh of browser.

* cleanup new FALAFEL_VERSION constant in falafel - using configurator everywhere instead of using constant directly

* fix prettier errors

* prettier server.ts

* sdk version to 0

* version is now a str like 2.1.0 in falafel and sdk. moved getRollupProviderStatus into SDK so that it doesn't need to accept sdk_version as arg

* prettier sdk index.ts

* zk-money alerts on version mismatch. Falafel allows requests without version header, but if version header is present it enforces a match.

* server rollup provider and block source shouldn't NEED a  to interact with falafel.

* prettier rollup provider ts

* block source / provider fix version default undefined changed to ''

Co-authored-by: joss-aztec <94053499+joss-aztec@users.noreply.github.com>
Co-authored-by: Charlie Lye <karl.lye@gmail.com>
Co-authored-by: Lasse Herskind <16536249+LHerskind@users.noreply.github.com>
Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com>
Co-authored-by: zac-williamson <blorktronics@gmail.com>
Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: Bug. Something is broken.
Projects
Archived in project
4 participants