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.

Sign up for GitHub

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

Jump to bottom

ManyChainMultiSig #517

Merged
merged 26 commits into from
Sep 19, 2024
Merged

ManyChainMultiSig #517

merged 26 commits into from
Sep 19, 2024

Conversation

augustbleeds
Copy link
Collaborator

@augustbleeds augustbleeds commented Aug 29, 2024 •
edited
Loading

  • contract done
  • set_config tests
  • set_root tests
  • execute tests

calvwang9
calvwang9 reviewed Sep 18, 2024
View reviewed changes
contracts/src/tests/test_mcms/test_execute.cairo Outdated Show resolved Hide resolved
contracts/src/mcms.cairo Outdated Show resolved Hide resolved
contracts/src/tests/test_mcms/test_set_config.cairo Outdated Show resolved Hide resolved
contracts/src/mcms.cairo Outdated
// encode the data field by looping through
let mut i = 0;
while i < op.data.len() {
encoded_leaf = encoded_leaf.encode(*op.data.at(i));
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok i think i understand now - i was confused as to how calldata is handled on Starknet.
Calldata on EVM and most other chains usually represents the encoded raw data to be passed in a transaction and is in the format of bytes or a hex string, e.g. EVM calldata is 0x + <4 bytes func selector> + <encoded args>. However on Starknet it seems like calldata is more akin to "raw args" rather than encoded data and is represented by an array of felts corresponding to args (or arg metadata). I realized that since there's no encoding necessary as it gets passed straight to the called contract, you're right - we are free to choose the encoding method here for the purposes of generating a hash for MCM and just have to make sure it is consistent between the contract and tooling.

calvwang9
calvwang9 reviewed Sep 18, 2024
View reviewed changes
Copy link
Contributor

@calvwang9 calvwang9 left a comment

Choose a reason for hiding this comment

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

nice to see in-depth tests 🎉 just minor comments

@augustbleeds
Copy link
Collaborator Author

thanks @calvwang9 👍 , i pushed a change to address all the comments

@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@augustbleeds augustbleeds merged commit 3e2d232 into develop Sep 19, 2024
22 checks passed
@augustbleeds augustbleeds deleted the augustus.mcms branch September 19, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calvwang9 calvwang9 calvwang9 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@augustbleeds @calvwang9