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

chore: cairo v2 migration #453

Merged
merged 25 commits into from
Jul 11, 2023
Merged

chore: cairo v2 migration #453

merged 25 commits into from
Jul 11, 2023

Conversation

Orland0x
Copy link
Contributor

@Orland0x Orland0x commented Jul 7, 2023

No description provided.

@Orland0x Orland0x marked this pull request as ready for review July 10, 2023 12:18
@Orland0x Orland0x requested a review from pscott July 10, 2023 13:47
Copy link
Contributor

@pscott pscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, just a few questions regarding u64 and packing :)

target: ContractAddress,
author: ContractAddress,
proposal_id: u256,
execution_strategy: Strategy
);
#[l1_handler]
fn commit(from_address: felt252, sender_address: felt252, hash: felt252);
// TODO: Should L1 handlers be part of the interface?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I originally thought "yes" but now I think it shouldn't because users will not be able to call this function anyway so I don't see the point of having it in the interface

Copy link
Contributor Author

@Orland0x Orland0x Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah thats currently what the compiler accepts. To me it seems like they should be included though, as its an external entrypoint to the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But only callable by a single address, enforced by the protcol itself so idk ?

starknet/src/utils/math.cairo Show resolved Hide resolved
fn ne(lhs: FinalizationStatus, rhs: FinalizationStatus) -> bool {
FinalizationStatusIntoU8::into(lhs) != rhs.into()
fn ne(lhs: @Strategy, rhs: @Strategy) -> bool {
!(lhs.clone() == rhs.clone())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sucks we need to clone here... I mean in theory we're just checking stuff, not modifying, so I'm guessing there should be a way of not cloning, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure. the impls in the standard lib dereference the snapshot: https://github.com/starkware-libs/cairo/blob/22ccda9eb66afcb6465e4778ddc310a5d09e3c91/corelib/src/integer.cairo#L297C9-L297C9 so doesnt look like there is a way to compare snapshots directly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for u8 yeah sure but for param it seems very expensive to do that.. but yeah maybe it's my Rust background talking here... :p

But like the logic is: if you're comparing values, you def don't need to take ownership of them :p

params: Array<felt252>,
}

/// NOTE: Using u64 for timestamps instead of u32 which we use in sx-evm. can change if needed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale for introducing this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its the default type returned by the syscall, not much reasoning atm. We were not packing the timestamps anyway right now so felt unnecessary to cast

starknet/src/utils/types.cairo Show resolved Hide resolved
value.active_voting_strategies
)
}

fn size_internal(value: Proposal) -> u8 {
9_u8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah now I see a second implementation, is it supposed to be the number of words ? (word being a felt252) ?
In that case, couldn't we pack all the u64 to reduce storage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we should, i assume this will be handled by the compiler/ another library soon though so not sure its worth writing our own impl

@Orland0x Orland0x merged commit 20ae041 into feat_space Jul 11, 2023
2 checks passed
Orland0x added a commit that referenced this pull request Jul 14, 2023
* Initial commit

* chore: repo setup with scarb + yarn

* feat: space state interface

* chore: updated package name

* feat: space constructor base

* chore: change repo structure

* feat: proposal struct

* feat: StorageAccess impl for Array<u8>

* feat: StorageAccess impl for strategy

* test: constructor base

* refactor: storage impls to not use syscalls directly

* fix: array storage impl runtime errors

* refactor: split utils and tests into dedicated files

* feat: set bit func

* feat: is bit set func

* feat: add voting strategies

* feat: derive option impl on Strategy

* chore: add bit packer to import path

* refactor: implement bit setter as a trait

* feat: add and remove authenticators

* refactor: use unwrap when adding voting strategies

* feat: updated constructor

* feat: StorageAccess impl on Proposal

* chore: use native pow func

* feat: u8 array to felt array impl

* refactor: u64 for typestamps

* feat: propose func

* feat: vanilla proposal validation strategy

* feat: IProposalValidationStrategy

* feat: call proposal validation strategy in propose

* chore: fixed typos

* test: deploy space with vanilla proposal validation

* chore: remove deps

* feat: space constructor calldata serialization

* chore: updated space interface

* chore: formatting

* chore: propose test

* feat: vanilla authenticator

* feat: vanilla execution strategy

* feat: proposals view func in space

* test: check proposal state

* feat: vanilla voting strategy

* fix: remove integer literals

* test: mock always fail proposal validation strategy

* chore: generate cairo_project.toml and use it for external deps

* chore: added OZ ownable as external lib

* feat: use OZ ownable in space

* feat: external setters with events

* chore formatting

* chore: added setters to space interface

* chore: fixed tests

* feat proposal creation event

* chore: cleaned up imports

* fix: voting strategy limit check

* chore: removed old starknet tooling

* chore: delete cairo_project.toml

* chore: add cairo_project.toml to gitignore

* feat: assert only authenticator

* fix: removed dummy values from propose func

* feat: IndexedStrategy type def and impl

* feat: move zeroable impl to math file and impl for u64

* fix: vanilla voting strategyname and return

* refactor: move module interfaces to dedicated files

* refactor: make modules implement their interfaces

* refactor: make space impl its interface

* feat(space): added missing interface implementations

* feat(space): transfer ownership

* feat(space): renounce ownership

* refactor: rename selector

* chore: formatting

* fix: bit setter

* feat: num executed getter in vanilla execution)

* fix: interface impls

* fix: add voting strategies assert

* chore: added propose selector to constants

* fix(authenticator): panic and propogate error when call fails

* feat: proper space setup in tests

* refactor: move constants out of module

* feat: vote func (#411)

* feat: vote func base

* chore: fixed name errors

* feat: get cumulative voting power func

* feat: choice enum and its LegacyHash impl

* feat: vote func with choice enum

* chore: test vote

---------

Co-authored-by: Orlando <[email protected]>

* fix: casing

* feat: execute function and simple quorum strategy (#413)

* chore: added execution payload to execution strategy interface

* feat: FinalizationStatus def and PartialEq, Into, TryInto, and StorageAccess impls

* feat: space execute function

* feat: simple quorum execution strategy and impl in vanilla execution strategy

* feat: Proposal Status enum and impls for Into and PartialEq traits

* chore: added execute to space interface

* feat: execute test and combined propose + vote + execute into single test

---------

Co-authored-by: Orlando <[email protected]>

* feat: update proposal (#414)

* feat(space): update proposal

* chore: test update proposal:

---------

Co-authored-by: Orlando <[email protected]>

* feat(space): cancel proposal

* fix: proposal timestamps

* feat: ParialEq for Proposal

* test: cancel proposal

* refactor: use felt arrays for all arbitrary data arrays (#417)

Co-authored-by: Orlando <[email protected]>

* feat: eth relayer execution strategy (#419)

* feat: eth relayer

* refactor: eth relayer relays entire proposal state to l1

---------

Co-authored-by: Orlando <[email protected]>

* feat: L1 avatar execution (#420)

* chore: added foundry project for ethereum code

* feat: legacy L1 avatar execution strategy

* feat: Simple Quorum L1 avatar execution

---------

Co-authored-by: Orlando <[email protected]>

* feat: eth tx authenticator (#423)

* feat: starknet commit contract

* feat: eth tx authenticator

* fix: add selectors to payload hash computation

* chore: gas instructions for vote func

---------

Co-authored-by: Orlando <[email protected]>

* feat: storage proof based voting strategy (#425)

* feat: single slot proof lib

* chore: updated imorts

* feat: timestamp to eth block number resolver

* feat: eth balance of voting strategy

---------

Co-authored-by: Orlando <[email protected]>

* feat: eth sig authenticator (#412)

Co-authored-by: Orlando <[email protected]>

* chore: removed alexandria dep

* chore: Cairo and Solidity contracts CI

* refactor: derive copy for Choice (#434)

* fix: use type inference (#445)

* refactor: reuse into traits (#446)

* Feat add set bit false (#441)

* feat: add set bit false

* feat: implement _remove_voting_strategies

* fix: remove typo line

* fix: check there is at least one active voting strategy after removal

* fix: rename valid to is_valid (#448)

* chore: cairo v2 migration (#453)

* chore: migrate ownable

* chore: migrate simple quorum

* chore: migrate vanilla execution

* chore: migrate eth relayer

* chore: migrate eexecution strategy interface

* chore: migrate authenticators

* chore: migrate execution strategies

* chore: migrate utils

* chore: migrate voting strategies

* chore: migrate interfaces

* chore: migrate proposal validation strategies

* chore: migrate space

* chore: update tests

* chore: allow all lib funcs in config

* chore: remove timestamp resolver

* chore: formatting

* chore: cleaned up CI workflow

* feat: PartialEq impl on Strategy

* chore: updated tests to cairo v2

* fix: remove voting strats errors

* chore: use StorageAccess impl for felt arrays from starknet by example

* refactor: use read and write at offset in read and write funcs for StorageAccess impls

* chore: formatting

* fix: merge error

---------

Co-authored-by: Orlando <[email protected]>

---------

Co-authored-by: Orlando <[email protected]>
Co-authored-by: pscott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants