Feat/add core contract deployer - #41
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new core-contract subcommand to the saya binary for managing Piltover core contract deployments. The subcommand provides functionality to declare the contract class, deploy instances, and configure program info and fact registry settings for settlement on Starknet.
Changes:
- Added
core_contractmodule with CLI commands for declaring, deploying, and setting up core contracts - Introduced constants for program hashes, initial state values, and network configuration
- Added utility function to compute Starknet OS config hash
- Updated dependencies to include dojo-utils and starknet-rs from feature branches
- Added comprehensive README documentation for using the new subcommand
Reviewed changes
Copilot reviewed 1 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| bin/saya/src/main.rs | Integrated CoreContract subcommand into main CLI |
| bin/saya/src/core_contract/mod.rs | Module declaration exposing CLI interface |
| bin/saya/src/core_contract/cli.rs | Core implementation with declare, deploy, and setup-program commands |
| bin/saya/src/core_contract/constants.rs | Network and program hash constants with documentation |
| bin/saya/src/core_contract/short_string.rs | Utility for computing SNOS config hash with test coverage |
| bin/saya/Cargo.toml | Added new dependencies for contract deployment functionality |
| README.md | Documentation for using the core-contract subcommand |
| Cargo.lock | Updated dependency resolution for new packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 15 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
scripts/celestia.sh:25
$Sudois used for the initdocker runcommand, but the script uses$SUDOelsewhere. As written,./scripts/celestia.sh initwill fail unless aSudoenv var happens to be set. Use a single variable name consistently (e.g.,$SUDO) and consider defaulting it to an empty string when unset.
export IMAGE=ghcr.io/celestiaorg/celestia-node:v0.28.4
export VOLUME=celestia-light-mocha
if [ "$1" = "init" ]; then
$Sudo docker run --rm -e NETWORK=$NETWORK -e RPC_URL=$RPC_URL -e RPC_PORT=$RPC_PORT \
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ``` | ||
| starkli invoke <PILTOVER_ADDRESS> set_facts_registry 0x01eda48cc753670a9a00313afd08bac6e1606943d554ea4a6040cd2953d67867 | ||
| ``` | ||
| cargo run core-contract --settlement-chain-id sepolia setup-program --core-contract-address <PILTOVER_ADDRESS> --chain-id example-chain --fact-registry-address 0x01eda48cc753670a9a00313afd08bac6e1606943d554ea4a6040cd2953d67867``` | ||
|
|
There was a problem hiding this comment.
The code fence here is malformed: the closing triple backticks are appended to the end of the command line, so the Markdown block won’t render correctly. Put the closing ``` on its own line (and consider formatting the long command over multiple lines for readability).
| use anyhow::Result; | ||
| use celestia_rpc::{BlobClient, Client, TxConfig}; | ||
| use celestia_types::{nmt::Namespace, AppVersion, Blob}; | ||
| use integrity::Felt; |
There was a problem hiding this comment.
CelestiaDataAvailabilityBackend is using integrity::Felt, but DataAvailabilityPointer::namespace is defined as a Starknet Felt (and other modules use starknet_types_core::felt::Felt). This will lead to type mismatches when assigning namespace and when passing the pointer through the orchestrator/settlement layers. Import and use the same Felt type as DataAvailabilityPointer.
| use integrity::Felt; | |
| use starknet_types_core::felt::Felt; |
| pub commitment: [u8; 32], | ||
| /// Celestia namespace ID. | ||
| pub namespace: Felt, | ||
| } |
There was a problem hiding this comment.
This module introduces DataAvailabilityPointer::namespace using starknet::core::types::Felt, but elsewhere in saya-core Felt is typically imported from starknet_types_core::felt::Felt (e.g. prover/mod.rs, settlement/mod.rs). Consider standardizing on one Felt import path to keep types consistent and avoid accidental cross-crate Felt mismatches (especially given celestia.rs currently imports a different Felt type).
No description provided.