feat: merge audit branch #1533

glihm · 2024-02-12T21:11:20Z

The audit branch contains several important changes:

The resource metadata are now manipulated as a model. But a special model that is automatically registered by sozo when the owner of the world deploys it.
The executor is no longer present. Now, the models are deployed to ensure there's no attack vector by executing the library_call syscall.
Add support for Array<felt252> in models.

Tasks:

Ensure the address of a model is always tracked by the manifest.
Verify how the array is handled into introspect and associated layout. SQL test are failing for this reason.
Add a link to the audit PDF on Nethermind public repository of audited codebase.

This commit aims at having more flexibility in the dojo storage engine, to automatically use several storage slots segments depending on the packed size. The offset and length fields are not more required for the database to work. The layout is still exposed for the moment.

There were a security issue with the executor, as currently the library_call is not immutable. Hence, a function called by library_call may contains other syscall. For this reason, the executor is removed in favor of deploying the model to get it's name.

glihm · 2024-02-14T17:51:29Z

I believe it comes from the "cainome" dependency. It's caused by tokio and maybe some other deps - gotta check if tokio is the only one. Tokio won't compile if the feature "full" is used. Only these features are supported on WASM

Appreciate the feedback on that, will double check cainome and possible sources based on that. Thanks mate. 🙏

tarrencev

some minor feedback from a initial pass

bin/sozo/src/commands/model.rs

bin/sozo/src/ops/migration/mod.rs

crates/dojo-core/src/base_test.cairo

crates/dojo-core/src/database_test.cairo

glihm · 2024-02-14T17:55:46Z

@ThePinion and @neotheprogramist if you have a chance I would appreciate a counter review on the core contracts changes. 🙏

@neotheprogramist you had started hard and great work on indexing, however for the audit it was removed as we should reconsider the model for this in a later phase. Appreciate your comprehension on that mate.

…mmand

crates/torii/migrations/20230316154230_setup.sql

codecov · 2024-02-14T21:48:44Z

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (374f2bf) 70.06% compared to head (082e369) 70.50%.
Report is 37 commits behind head on main.

Files	Patch %	Lines
crates/dojo-lang/src/inline_macros/array_cap.rs	73.91%	24 Missing ⚠️
crates/torii/grpc/src/server/mod.rs	0.00%	12 Missing ⚠️
crates/dojo-lang/src/introspect.rs	86.66%	10 Missing ⚠️
crates/torii/core/src/processors/register_model.rs	40.00%	9 Missing ⚠️
bin/sozo/src/ops/model.rs	0.00%	7 Missing ⚠️
crates/torii/core/src/model.rs	63.63%	4 Missing ⚠️
crates/dojo-world/src/contracts/model.rs	83.33%	3 Missing ⚠️
crates/dojo-world/src/manifest.rs	86.36%	3 Missing ⚠️
crates/dojo-world/src/migration/mod.rs	0.00%	3 Missing ⚠️
crates/dojo-lang/src/plugin.rs	91.30%	2 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1533      +/-   ##
==========================================
+ Coverage   70.06%   70.50%   +0.43%     
==========================================
  Files         236      261      +25     
  Lines       22531    25300    +2769     
==========================================
+ Hits        15786    17837    +2051     
- Misses       6745     7463     +718

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

glihm

Some comments on the sozo capabilities for backward compatibilities.
Thanks guys for the feedback. 👍

bin/sozo/src/commands/migrate.rs

bin/sozo/src/ops/migration/mod.rs

kariy · 2024-02-17T06:16:01Z

crates/dojo-lang/src/plugin.rs

+        let do_expand: bool =
+            std::env::var(DOJO_PLUGIN_EXPAND_VAR_ENV).map_or(false, |v| v == "true" || v == "1");
+


What's the purpose behind this env var? Is there a reason to print out the expanded code ?

Debugging mostly.

While working on dojo-lang to introduce the array, I found that having the compiler rewritten node expanded was very useful.

crates/dojo-world/src/migration/mod.rs

crates/dojo-test-utils/build.rs

glihm added 30 commits January 17, 2024 20:32

fix: ensure world metadata are handled safely with ACL

f129edf

fix: remove unused EXECUTE_ENTRYPOINT

73d7809

fix: add support for array for feature parity on metadata uri

5016cfe

fix: ensure the new class hash to upgrade to has world() entrypoint

8ff011f

wip: metadata rework

787d8c8

feat: add expand env var for plugin debug

597c436

fix: adjust world entry points based on storage rework

30e79c8

fix: remove unused WhereClause

98ed168

fix: ensure deployed contracts addresses don't conflict with model names

bfd952b

fix: remove TODO with expected behavior for packing

b1e1aa8

fix: ensure fpow is never used with base equal to 0 in our case

0c0a4ec

fix: remove old boolean enum usage

fdebe59

fix: ensure executor address is not zero

c804b77

fix: ensure packing max bits max value

29621f5

fix: remove unused file

88ccdde

fix: remove database/utils not used for now

ec03721

fix: remove index related code

e95153c

fix: ensure empty data are well retrieved from storage

f548b79

fix: remove the length from the storage to be backward compatible

4b5c0f8

fix: remove unused variable

9a94a39

fix: adjust gas for test

0cd0031

feat: add function to verify ownership of tx account

1d65123

fix: keep class hash and contract address for models

04f7ef7

feat: update toolchain for new model registration

cc534af

fix: revise world documentation and refacto world errors

ef92799

fix: remove unused storage variable

c04ffa5

fix: ensure the world address is protected against model registration

202d077

fix: remove unused variable

572fdc8

docs: fix README audit format

b9e385a

tarrencev reviewed Feb 14, 2024

View reviewed changes

glihm requested review from broody, piniom and neotheprogramist February 14, 2024 17:56

glihm added 2 commits February 14, 2024 12:15

chore: use new tag of cainome wasm compatible + remove unused variables

786e672

fix: fmt

7a79cf9

glihm requested a review from kariy February 14, 2024 18:37

glihm added 5 commits February 14, 2024 12:53

fix: add array as supported enum for models children ty

d9f57b2

fix(dojo-core): remove print statements not required

28a083f

fix: remove print for database but keep it for base_test address

2c91bc7

fix: add sozo command for class hash back and add contract address co…

0456f4c

…mmand

ci: avoid rate limitation for protoc

1575b14

broody reviewed Feb 14, 2024

View reviewed changes

crates/torii/migrations/20230316154230_setup.sql Outdated Show resolved Hide resolved

glihm added 5 commits February 14, 2024 14:38

fix(torii): ensure model contract address is in a new migration

f2bdd07

fix(torii): remove array indexing support for now

6619d6c

fix(dojo-core): disable use of capacity derive on model

589c9a4

fix: fmt

c54c7c0

fix(dojo-lang): run missing CAIRO_FIX tests

bd55004

glihm commented Feb 16, 2024

View reviewed changes

bin/sozo/src/commands/migrate.rs Outdated Show resolved Hide resolved

bin/sozo/src/ops/migration/mod.rs Outdated Show resolved Hide resolved

glihm requested a review from tarrencev February 16, 2024 04:01

glihm force-pushed the merge-audit branch from da2a8fe to bd55004 Compare February 16, 2024 16:20

kariy reviewed Feb 17, 2024

View reviewed changes

tarrencev approved these changes Feb 17, 2024

View reviewed changes

fix: remove debug print

082e369

glihm merged commit 072a31d into dojoengine:main Feb 18, 2024
12 checks passed

glihm mentioned this pull request Feb 23, 2024

[Core] Support Arrays #1417

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: merge audit branch #1533

feat: merge audit branch #1533

glihm commented Feb 12, 2024 •

edited

Loading

glihm commented Feb 14, 2024

tarrencev left a comment

glihm commented Feb 14, 2024

codecov bot commented Feb 14, 2024 •

edited

Loading

glihm left a comment

kariy Feb 17, 2024

glihm Feb 17, 2024

		let do_expand: bool =
		std::env::var(DOJO_PLUGIN_EXPAND_VAR_ENV).map_or(false, \|v\| v == "true" \|\| v == "1");

feat: merge audit branch #1533

feat: merge audit branch #1533

Conversation

glihm commented Feb 12, 2024 • edited Loading

glihm commented Feb 14, 2024

tarrencev left a comment

Choose a reason for hiding this comment

glihm commented Feb 14, 2024

codecov bot commented Feb 14, 2024 • edited Loading

Codecov Report

glihm left a comment

Choose a reason for hiding this comment

kariy Feb 17, 2024

Choose a reason for hiding this comment

glihm Feb 17, 2024

Choose a reason for hiding this comment

glihm commented Feb 12, 2024 •

edited

Loading

codecov bot commented Feb 14, 2024 •

edited

Loading