From a2de330b30b4bbebe575d7e834e74cb718cfd76e Mon Sep 17 00:00:00 2001 From: Laith Bahodi <70682032+hydrobeam@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:52:53 -0500 Subject: [PATCH 1/6] ci: upload contracts to CF R2 from main (#100) Co-authored-by: ahramy --- .github/workflows/pre-release.yaml | 66 ++++++++++++++++++ .github/workflows/release.yaml | 47 ++++++------- .github/workflows/reusable-build.yaml | 67 +++++++++++++++++++ ...build-upload.yaml => reusable-upload.yaml} | 59 +++++++--------- 4 files changed, 180 insertions(+), 59 deletions(-) create mode 100644 .github/workflows/pre-release.yaml create mode 100644 .github/workflows/reusable-build.yaml rename .github/workflows/{reusable-build-upload.yaml => reusable-upload.yaml} (82%) diff --git a/.github/workflows/pre-release.yaml b/.github/workflows/pre-release.yaml new file mode 100644 index 00000000..4b11275f --- /dev/null +++ b/.github/workflows/pre-release.yaml @@ -0,0 +1,66 @@ +# Workflow to fetch the latest commit hash on the main branch and upload artifacts to CF storage. +name: Build and upload from main +on: + workflow_dispatch: + +concurrency: ${{ github.workflow }}-${{ github.ref }} + +jobs: + stellar-contract-names: + name: Get all Stellar contracts + runs-on: blacksmith-2vcpu-ubuntu-2204 + outputs: + releases: ${{ steps.prepare-release.outputs.releases }} + commit_hash: ${{ steps.get-commit-hash.outputs.hash }} + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install dependencies + run: sudo apt-get install -y jq + + - name: Get latest commit hash + id: get-commit-hash + run: echo "hash=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT" + + - name: Prepare JSON output for release + id: prepare-release + run: | + RELEASES_JSON=$(find contracts -maxdepth 1 -mindepth 1 -type d | sed 's|contracts/||' | jq -R . | jq -s --arg commit "${{ steps.get-commit-hash.outputs.hash }}" 'map({ + package_name: ., + version: $commit, + package_git_tag: "\(.)_\($commit)" + })') + echo "releases=$(echo "$RELEASES_JSON" | jq -c)" >> "$GITHUB_OUTPUT" + + build: + needs: stellar-contract-names + uses: ./.github/workflows/reusable-build.yaml + with: + commit-hash: ${{ needs.stellar-contract-names.outputs.commit_hash }} + + upload: + needs: [stellar-contract-names, build] + strategy: + matrix: + releases: ${{ fromJson(needs.stellar-contract-names.outputs.releases) }} + + uses: ./.github/workflows/reusable-upload.yaml + permissions: + id-token: write + contents: read + with: + package-name: ${{ matrix.releases.package_name }} + package-version: ${{ matrix.releases.version }} + package-git-tag: ${{ matrix.releases.package_git_tag }} + artifact-name: ${{ needs.build.outputs.artifact-name }} + artifact-path: ${{ needs.build.outputs.artifact-path }} + cf-bucket-name: ${{ vars.CF_BUCKET_NAME }} + cf-config-bucket-root-key: ${{ vars.CF_BUCKET_ROOT_KEY }} + github-release: false + secrets: + github-token: ${{ secrets.PAT_TOKEN }} + cf-endpoint-url: ${{ secrets.CF_ENDPOINT_URL }} + cf-bucket-access-key-id: ${{ secrets.CF_BUCKET_ACCESS_KEY_ID }} + cf-bucket-secret-access-key: ${{ secrets.CF_BUCKET_SECRET_ACCESS_KEY }} diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c1778982..0e258b7d 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -6,7 +6,7 @@ on: pull_request: branches: - main - - 'releases/**' + - "releases/**" types: [closed] workflow_dispatch: @@ -14,7 +14,6 @@ on: concurrency: ${{ github.workflow }}-${{ github.ref }} jobs: - # Publishes a release in case the release isn't published publish-release: name: Publish releases @@ -24,21 +23,25 @@ jobs: ((github.event.pull_request.merged == true) && contains(github.event.pull_request.labels.*.name, 'release')) - runs-on: blacksmith-8vcpu-ubuntu-2204 + runs-on: blacksmith-2vcpu-ubuntu-2204 outputs: releases: ${{ steps.prepare-matrix.outputs.releases }} + commit_hash: ${{ steps.get-commit-hash.outputs.hash }} steps: - name: Checkout repository uses: actions/checkout@v4 with: - fetch-depth: 0 token: ${{ secrets.PAT_TOKEN }} - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable + - name: Get commit hash + id: get-commit-hash + run: echo "hash=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + # Creates git tags and publishes the crates of the new releases - name: Publish release id: publish-release @@ -56,35 +59,33 @@ jobs: run: | echo "releases=$(echo '${{ steps.publish-release.outputs.releases }}' | jq -c '.')" >> $GITHUB_OUTPUT - # Creates other artifacts needed (`wasm` files) - build-and-upload: - name: Build artifacts for ${{ matrix.releases.package_name }}-v${{ matrix.releases.version }} + build: needs: publish-release + uses: ./.github/workflows/reusable-build.yaml + with: + commit-hash: ${{ needs.publish-release.outputs.commit_hash }} - # Once a release is done for a package, we iterate on each of these packages and build its corresponding artifacts and upload them + upload: + needs: [publish-release, build] strategy: matrix: releases: ${{ fromJson(needs.publish-release.outputs.releases) }} - uses: ./.github/workflows/reusable-build-upload.yaml - + uses: ./.github/workflows/reusable-upload.yaml permissions: id-token: write contents: read - with: - package-name: "${{ matrix.releases.package_name }}" - package-version: "${{ matrix.releases.version }}" - package-git-tag: "${{ matrix.releases.tag }}" - - # CF Bucket related variables - cf-bucket-name: "${{ vars.CF_BUCKET_NAME }}" - - # The root key to be used for accessing the configs. (ex: `test-root-key` puts releases in `test-root-key/*`) - cf-config-bucket-root-key: "${{ vars.CF_BUCKET_ROOT_KEY }}" - + package-name: ${{ matrix.releases.package_name }} + package-version: ${{ matrix.releases.version }} + package-git-tag: ${{ matrix.releases.tag }} + artifact-name: ${{ needs.build.outputs.artifact-name }} + artifact-path: ${{ needs.build.outputs.artifact-path }} + cf-bucket-name: ${{ vars.CF_BUCKET_NAME }} + cf-config-bucket-root-key: ${{ vars.CF_BUCKET_ROOT_KEY }} + github-release: true secrets: - github-token: "${{ secrets.PAT_TOKEN }}" - cf-endpoint-url: "${{ secrets.CF_ENDPOINT_URL }}" + github-token: ${{ secrets.PAT_TOKEN }} + cf-endpoint-url: ${{ secrets.CF_ENDPOINT_URL }} cf-bucket-access-key-id: ${{ secrets.CF_BUCKET_ACCESS_KEY_ID }} cf-bucket-secret-access-key: ${{ secrets.CF_BUCKET_SECRET_ACCESS_KEY }} diff --git a/.github/workflows/reusable-build.yaml b/.github/workflows/reusable-build.yaml new file mode 100644 index 00000000..17b7a27f --- /dev/null +++ b/.github/workflows/reusable-build.yaml @@ -0,0 +1,67 @@ +name: "Build Contracts" + +on: + workflow_call: + inputs: + commit-hash: + description: "The commit hash to build from" + type: string + required: true + outputs: + artifact-name: + description: "Name of the uploaded artifact containing all builds" + value: ${{ jobs.build.outputs.artifact-name }} + artifact-path: + description: "Path of the uploaded artifact containing all builds" + value: ${{ jobs.build.outputs.artifact-path }} + +jobs: + build: + runs-on: blacksmith-8vcpu-ubuntu-2204 + outputs: + artifact-name: ${{ steps.set-artifact-name.outputs.name }} + artifact-path: ${{ steps.set-artifact-name.outputs.path }} + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Checkout specific commit + run: git checkout ${{ inputs.commit-hash }} + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: 1.81.0 + targets: wasm32-unknown-unknown + + - name: Set artifact name + id: set-artifact-name + run: | + echo "name=wasm-builds-${{ inputs.commit-hash }}" >> $GITHUB_OUTPUT + echo "path=target/wasm32-unknown-unknown/release" >> $GITHUB_OUTPUT + + - name: Build all contracts + run: | + # Install Stellar CLI compatible with the soroban-sdk version in Cargo.toml + cargo install --locked stellar-cli --version ^22 --features opt + + # Build all contracts + stellar contract build + ./optimize.sh + + # Process in the release directory + cd target/wasm32-unknown-unknown/release + + # Remove unoptimized files and rename optimized ones + # This ensures we only keep the optimized versions + find . -type f -name "*.wasm" ! -name "*.optimized.wasm" -maxdepth 1 -delete + find . -name "*.optimized.wasm" -maxdepth 1 -exec sh -c 'mv "$0" "${0%.optimized.wasm}.wasm"' {} \; + find . -type f ! -name "*.wasm" -delete + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.set-artifact-name.outputs.name }} + path: target/wasm32-unknown-unknown/release + retention-days: 1 diff --git a/.github/workflows/reusable-build-upload.yaml b/.github/workflows/reusable-upload.yaml similarity index 82% rename from .github/workflows/reusable-build-upload.yaml rename to .github/workflows/reusable-upload.yaml index ee6b103c..313e04c1 100644 --- a/.github/workflows/reusable-build-upload.yaml +++ b/.github/workflows/reusable-upload.yaml @@ -1,37 +1,43 @@ -name: "Publish specific rust package" +name: "Upload Contract wasm to Cloudflare R2" on: workflow_call: inputs: - # Package related variables package-name: description: "The package name to use (ex: gz-srv)" type: string required: true default: "" - package-version: description: "The package version to use (ex: 0.1.2)" type: string required: true default: "" - package-git-tag: description: "the release tag name (ex: gz-srv-v0.1.2)" type: string required: true default: "" - - # CF Bucket related variables + artifact-name: + description: "Name of the artifact containing the builds" + type: string + required: true + artifact-path: + description: "Path of the artifact containing the builds" + type: string + required: true cf-bucket-name: description: "The CF bucket name to use" required: true type: string - cf-config-bucket-root-key: description: "The root key to be used for accessing the configs. (ex: `test-root-key` puts releases in `test-root-key/*`)" required: true type: string + github-release: + description: "Whether to upload as a github release" + type: boolean + default: true secrets: github-token: @@ -48,36 +54,22 @@ on: required: true jobs: - build-and-upload: - runs-on: blacksmith-8vcpu-ubuntu-2204 + upload: + name: Upload ${{ inputs.package-git-tag }} + runs-on: blacksmith-2vcpu-ubuntu-2204 steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 0 - token: ${{ secrets.github-token }} - - - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@stable + - name: Download artifact + uses: actions/download-artifact@v4 with: - toolchain: 1.81.0 - targets: wasm32-unknown-unknown + name: ${{ inputs.artifact-name }} + path: ${{ inputs.artifact-path }} - - name: Build artifacts for ${{ inputs.package-name }}-v${{ inputs.package-version }} - run: | - echo "Building wasm for '${{ inputs.package-name }}-v${{ inputs.package-version }}'"; - cargo install --locked stellar-cli --version 22.2.0 --features opt - cargo wasm -p ${{ inputs.package-name }} - stellar contract build - ./optimize.sh - - # Prepare the variables that will be used across the different next steps - name: Prepare cross-steps variables run: | export PACKAGE_NAME='${{ inputs.package-name }}' - export PACKAGE_VERSION='v${{ inputs.package-version }}' + export PACKAGE_VERSION=${{ inputs.github-release && format('v{0}', inputs.package-version) || inputs.package-version }} - export BASE_ARTIFACTS_DIR="./target/wasm32-unknown-unknown/release" + export BASE_ARTIFACTS_DIR='${{ inputs.artifact-path }}' export ARTIFACT_NAME="axelar-cgp-stellar-wasm-${PACKAGE_NAME}-${PACKAGE_VERSION}" export BASE_ARTIFACTS_VERSIONED_DIR="$(dirname ${BASE_ARTIFACTS_DIR})/${ARTIFACT_NAME}" # Regardless of the dir type, relative or absolute @@ -115,12 +107,6 @@ jobs: # This cd to keep the dir structure of the artifacts archive cd ${{ env.BASE_ARTIFACTS_VERSIONED_DIR }} - # Remove "unoptimized" built wasm files - find "." -type f -name "*.wasm" ! -name "*.optimized.wasm" -maxdepth 1 -delete - - # Rename the optimized ones and remove the ".optimized" suffix - find . -name "*.optimized.wasm" -maxdepth 1 -exec sh -c 'mv "$0" "${0%.optimized.wasm}.wasm"' {} \; - # Archive the wasm find "." -type f -name "*.wasm" -maxdepth 1 -print | zip "${{ env.ZIP_ARCHIVE_FILE }}" -@ find "." -type f -name "*.wasm" -maxdepth 1 -print | tar -czvf "${{ env.TAR_ARCHIVE_FILE }}" -T - @@ -176,6 +162,7 @@ jobs: # https://github.com/orgs/community/discussions/26263#discussioncomment-3251069 - name: Update the GitHub Release uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191 # v2.0.8 + if: inputs.github-release with: tag_name: ${{ inputs.package-git-tag }} # This uses the tag from the push files: | From cbbc41005435baf20809c892b196f468c55b84d1 Mon Sep 17 00:00:00 2001 From: Milap Sheth Date: Tue, 4 Feb 2025 20:39:37 -0500 Subject: [PATCH 2/6] docs: fix docs publish action (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Noah B. 🥂 <15343884+nbayindirli@users.noreply.github.com> --- .cargo/config.toml | 3 +++ .github/workflows/upload-docs.yaml | 13 ++++++++----- .../stellar-axelar-gas-service/src/interface.rs | 4 ++-- contracts/stellar-axelar-operators/src/interface.rs | 4 ++-- .../src/executable.rs | 6 +++--- .../src/interface.rs | 6 +++--- contracts/stellar-interchain-token/src/interface.rs | 9 ++++++--- contracts/stellar-token-manager/src/interface.rs | 2 +- packages/stellar-axelar-std-derive/src/lib.rs | 5 ++--- 9 files changed, 30 insertions(+), 22 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index afc7d59a..b4990beb 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,6 @@ [alias] wasm = "build --release --lib --target wasm32-unknown-unknown --locked --workspace" unit-test = "test --lib" + +[build] +rustdocflags = ["-D", "warnings"] diff --git a/.github/workflows/upload-docs.yaml b/.github/workflows/upload-docs.yaml index 338672b7..51b72f15 100644 --- a/.github/workflows/upload-docs.yaml +++ b/.github/workflows/upload-docs.yaml @@ -39,19 +39,22 @@ jobs: cargo doc --no-deps --workspace # Create an index.html that redirects to the main crate's docs - echo '' > target/doc/index.html + echo '' > target/doc/index.html # Copy the generated docs to the pages directory - mkdir -p pages - cp -r target/doc/* pages/ + mkdir -p docs + cp -r target/doc/* docs/ - name: Setup Pages uses: actions/configure-pages@v5 + - name: Build with Jekyll + uses: actions/jekyll-build-pages@v1 + with: + source: ./docs + - name: Upload artifact uses: actions/upload-pages-artifact@v3 - with: - path: pages deploy: needs: build diff --git a/contracts/stellar-axelar-gas-service/src/interface.rs b/contracts/stellar-axelar-gas-service/src/interface.rs index c7a97a6a..244e84a8 100644 --- a/contracts/stellar-axelar-gas-service/src/interface.rs +++ b/contracts/stellar-axelar-gas-service/src/interface.rs @@ -74,7 +74,7 @@ pub trait AxelarGasServiceInterface: OperatableInterface { /// - [`ContractError::InsufficientBalance`]: If the contract's token balance is insufficient to cover the transfer. /// /// # Authorization - /// - [`Self::operator`] must authorize. + /// - [`OperatableInterface::operator`] must authorize. fn collect_fees(env: Env, receiver: Address, token: Token) -> Result<(), ContractError>; /// Refunds gas payment to the specified receiver in relation to a specific cross-chain message. @@ -85,6 +85,6 @@ pub trait AxelarGasServiceInterface: OperatableInterface { /// * `token` - The token used for the refund, including the address and amount. /// /// # Authorization - /// - [`Self::operator`] must authorize. + /// - [`OperatableInterface::operator`] must authorize. fn refund(env: Env, message_id: String, receiver: Address, token: Token); } diff --git a/contracts/stellar-axelar-operators/src/interface.rs b/contracts/stellar-axelar-operators/src/interface.rs index a2caa1aa..e6b6c850 100644 --- a/contracts/stellar-axelar-operators/src/interface.rs +++ b/contracts/stellar-axelar-operators/src/interface.rs @@ -22,7 +22,7 @@ pub trait AxelarOperatorsInterface: OwnableInterface { /// - [`ContractError::OperatorAlreadyAdded`]: If the specified account is already an operator. /// /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn add_operator(env: Env, account: Address) -> Result<(), ContractError>; /// Remove an address as an operator. @@ -36,7 +36,7 @@ pub trait AxelarOperatorsInterface: OwnableInterface { /// - [`ContractError::NotAnOperator`]: If the specified account is not an operator. /// /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn remove_operator(env: Env, account: Address) -> Result<(), ContractError>; /// Execute a function on any contract as the operators contract. diff --git a/contracts/stellar-interchain-token-service/src/executable.rs b/contracts/stellar-interchain-token-service/src/executable.rs index 7c415f17..174c3f49 100644 --- a/contracts/stellar-interchain-token-service/src/executable.rs +++ b/contracts/stellar-interchain-token-service/src/executable.rs @@ -3,7 +3,7 @@ //! This is an executable interface that accepts an interchain token from ITS contract //! along with an arbitrary message. //! -//! This is similar to the [AxelarExecutableInterface] but meant for messages sent with an ITS token. +//! This is similar to the [`AxelarExecutableInterface`](stellar_axelar_gateway::executable::AxelarExecutableInterface) but meant for messages sent with an ITS token. use soroban_sdk::{contractclient, Address, Bytes, BytesN, Env, String}; pub use stellar_axelar_std::InterchainTokenExecutable; @@ -11,7 +11,7 @@ pub use stellar_axelar_std::InterchainTokenExecutable; /// This trait must be implemented by a contract to be compatible with the [`InterchainTokenExecutableInterface`]. /// /// To make a contract executable by the interchain token service contract, it must implement the [`InterchainTokenExecutableInterface`] trait. -/// For security purposes and convenience, sender authorization and other commonly shared code necessary to implement that trait can be automatically generated with the [`axelar_soroban_std::Executable`] derive macro. +/// For security purposes and convenience, sender authorization and other commonly shared code necessary to implement that trait can be automatically generated with the [`InterchainTokenExecutable`] derive macro. /// All parts that are specific to an individual ITS executable contract are collected in this [`CustomInterchainTokenExecutable`] trait and must be implemented by the contract to be compatible with the [`InterchainTokenExecutableInterface`] trait. /// /// Do NOT add the implementation of [`CustomInterchainTokenExecutable`] to the public interface of the contract, i.e. do not annotate the `impl` block with `#[contractimpl]` @@ -36,7 +36,7 @@ pub trait CustomInterchainTokenExecutable { ) -> Result<(), Self::Error>; } -/// Interface for an Interchain Token Executable app. Use the [`stellar_axelar_std::Executable`] derive macro to implement this interface. +/// Interface for an Interchain Token Executable app. Use the [`InterchainTokenExecutable`] derive macro to implement this interface. /// /// **DO NOT IMPLEMENT THIS MANUALLY!** #[contractclient(name = "InterchainTokenExecutableClient")] diff --git a/contracts/stellar-interchain-token-service/src/interface.rs b/contracts/stellar-interchain-token-service/src/interface.rs index 86f95284..43790a81 100644 --- a/contracts/stellar-interchain-token-service/src/interface.rs +++ b/contracts/stellar-interchain-token-service/src/interface.rs @@ -44,12 +44,12 @@ pub trait InterchainTokenServiceInterface: /// Sets the specified chain as trusted for cross-chain messaging. /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn set_trusted_chain(env: &Env, chain: String) -> Result<(), ContractError>; /// Removes the specified chain from trusted chains. /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn remove_trusted_chain(env: &Env, chain: String) -> Result<(), ContractError>; /// Computes the unique identifier for an interchain token. @@ -128,7 +128,7 @@ pub trait InterchainTokenServiceInterface: /// - [`ContractError::InvalidFlowLimit`]: If the provided flow limit is not positive. /// /// # Authorization - /// - [`Self::operator`] must authorize. + /// - [`OperatableInterface::operator`] must authorize. fn set_flow_limit( env: &Env, token_id: BytesN<32>, diff --git a/contracts/stellar-interchain-token/src/interface.rs b/contracts/stellar-interchain-token/src/interface.rs index 2fb0a984..2711552d 100644 --- a/contracts/stellar-interchain-token/src/interface.rs +++ b/contracts/stellar-interchain-token/src/interface.rs @@ -1,11 +1,14 @@ use soroban_sdk::token::{self, StellarAssetInterface}; use soroban_sdk::{contractclient, Address, BytesN, Env}; +use stellar_axelar_std::interfaces::OwnableInterface; use crate::error::ContractError; #[allow(dead_code)] #[contractclient(name = "InterchainTokenClient")] -pub trait InterchainTokenInterface: token::Interface + StellarAssetInterface { +pub trait InterchainTokenInterface: + token::Interface + StellarAssetInterface + OwnableInterface +{ /// Returns the Interchain Token ID fn token_id(env: &Env) -> BytesN<32>; @@ -38,7 +41,7 @@ pub trait InterchainTokenInterface: token::Interface + StellarAssetInterface { /// * `minter` - The address to be added as a minter. /// /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn add_minter(env: &Env, minter: Address); /// Removes a new minter from the Interchain Token contract. @@ -47,6 +50,6 @@ pub trait InterchainTokenInterface: token::Interface + StellarAssetInterface { /// * `minter` - The address to be added as a minter. /// /// # Authorization - /// - [`Self::owner`] must authorize. + /// - [`OwnableInterface::owner`] must authorize. fn remove_minter(env: &Env, minter: Address); } diff --git a/contracts/stellar-token-manager/src/interface.rs b/contracts/stellar-token-manager/src/interface.rs index fa133673..d5a2346d 100644 --- a/contracts/stellar-token-manager/src/interface.rs +++ b/contracts/stellar-token-manager/src/interface.rs @@ -17,7 +17,7 @@ pub trait TokenManagerInterface: OwnableInterface + UpgradableInterface { /// - `Ok(Val)` - The result of the function execution. /// /// # Authorization - /// - [`Self::owner`] must have authorized. + /// - [`OwnableInterface::owner`] must have authorized. fn execute( env: &Env, contract: Address, diff --git a/packages/stellar-axelar-std-derive/src/lib.rs b/packages/stellar-axelar-std-derive/src/lib.rs index f5c11ff3..99eaac33 100644 --- a/packages/stellar-axelar-std-derive/src/lib.rs +++ b/packages/stellar-axelar-std-derive/src/lib.rs @@ -98,9 +98,8 @@ pub fn derive_pausable(input: TokenStream) -> TokenStream { /// /// # Example /// ```rust,ignore -/// # mod test { -/// # use soroban_sdk::{contract, contractimpl, Address, Env}; -/// use stellar_axelar_std_derive::when_not_paused; +/// # use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; +/// use stellar_axelar_std::{Pausable, when_not_paused}; /// /// #[contracttype] /// pub enum ContractError { From bdc02e640c07a81758e487269a5473fcccf54b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Noah=20B=2E=20=F0=9F=A5=82?= <15343884+nbayindirli@users.noreply.github.com> Date: Thu, 6 Feb 2025 08:41:42 -0800 Subject: [PATCH 3/6] refactor(axelar-std-derive): simplify contractstorage macro (#241) --- contracts/stellar-example/src/contract.rs | 6 +- ...ata_key_storage_schema_is_unchanged.golden | 25 +- packages/stellar-axelar-std-derive/Cargo.toml | 4 +- .../stellar-axelar-std-derive/src/storage.rs | 576 ++++++++++-------- .../storage_schema_generation_succeeds.golden | 143 +++-- .../stellar-axelar-std/src/tests/storage.rs | 68 ++- ...ata_key_storage_schema_is_unchanged.golden | 34 +- 7 files changed, 538 insertions(+), 318 deletions(-) diff --git a/contracts/stellar-example/src/contract.rs b/contracts/stellar-example/src/contract.rs index 373d8576..97d5f72e 100644 --- a/contracts/stellar-example/src/contract.rs +++ b/contracts/stellar-example/src/contract.rs @@ -34,7 +34,7 @@ impl AxelarExecutableInterface for Example { type Error = ExampleError; fn gateway(env: &Env) -> Address { - storage::gateway(env).expect("gateway not found") + storage::gateway(env) } fn execute( @@ -62,7 +62,7 @@ impl CustomInterchainTokenExecutable for Example { type Error = ExampleError; fn __interchain_token_service(env: &Env) -> Address { - storage::interchain_token_service(env).expect("interchain token service not found") + storage::interchain_token_service(env) } fn __authorized_execute_with_token( @@ -118,7 +118,7 @@ impl Example { #[contractimpl] impl ExampleInterface for Example { fn gas_service(env: &Env) -> Address { - storage::gas_service(env).expect("gas service not found") + storage::gas_service(env) } fn send( diff --git a/contracts/stellar-example/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden b/contracts/stellar-example/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden index de9f414e..4333e0e0 100644 --- a/contracts/stellar-example/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden +++ b/contracts/stellar-example/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden @@ -1,6 +1,19 @@ -#[derive(Clone, Debug)] enum DataKey -{ - #[instance] #[value(Address)] Gateway, #[instance] #[value(Address)] - GasService, #[instance] #[value(Address)] InterchainTokenService, - #[instance] #[status] Paused, -} \ No newline at end of file +#[derive(Clone, Debug)] +enum DataKey { + + #[instance] + #[value(Address)] + Gateway, + + #[instance] + #[value(Address)] + GasService, + + #[instance] + #[value(Address)] + InterchainTokenService, + + #[instance] + #[status] + Paused, +} diff --git a/packages/stellar-axelar-std-derive/Cargo.toml b/packages/stellar-axelar-std-derive/Cargo.toml index 9eacd6e2..6a396a6b 100644 --- a/packages/stellar-axelar-std-derive/Cargo.toml +++ b/packages/stellar-axelar-std-derive/Cargo.toml @@ -14,13 +14,13 @@ proc-macro = true [dependencies] heck = "0.5" itertools = "0.14" +prettyplease = { version = "0.2", default-features = false } proc-macro2 = { workspace = true } quote = { version = "1.0", default-features = false } -syn = { version = "2.0", features = ["full", "extra-traits"] } +syn = { version = "2.0", default-features = false, features = ["derive", "extra-traits", "full", "parsing"] } [dev-dependencies] goldie = { workspace = true } -prettyplease = "0.2" [lints] workspace = true diff --git a/packages/stellar-axelar-std-derive/src/storage.rs b/packages/stellar-axelar-std-derive/src/storage.rs index 9f4a4738..8901f9df 100644 --- a/packages/stellar-axelar-std-derive/src/storage.rs +++ b/packages/stellar-axelar-std-derive/src/storage.rs @@ -1,5 +1,8 @@ +use std::convert::TryFrom; + use heck::ToSnakeCase; use itertools::Itertools; +use prettyplease::unparse; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; use syn::{Attribute, Data, DataEnum, DeriveInput, Fields, FieldsNamed, Meta, Type, Variant}; @@ -9,9 +12,238 @@ enum Value { Type(Type), } -struct StorageAttributes { - storage_type: StorageType, - value: Value, +impl TryFrom<&[Attribute]> for Value { + type Error = String; + + fn try_from(attrs: &[Attribute]) -> Result { + let attr = attrs + .iter() + .filter(|attr| attr.path().is_ident("status") || attr.path().is_ident("value")) + .exactly_one() + .map_err(|_| "exactly one of #[status] and #[value(Type)] must be provided")?; + + if attr.path().is_ident("status") { + Ok(Self::Status) + } else if let Meta::List(list) = &attr.meta { + Ok(Self::Type( + list.parse_args::() + .map_err(|_| "failed to parse value type")?, + )) + } else { + Err("value attribute must contain a type parameter: #[value(Type)]".into()) + } + } +} + +trait FieldsExt { + fn names(&self) -> Vec<&Ident>; + + fn types(&self) -> Vec<&Type>; +} + +impl FieldsExt for Fields { + /// Returns the field names of a storage enum variant. + fn names(&self) -> Vec<&Ident> { + match self { + Self::Unit => vec![], + Self::Named(fields) => fields + .named + .iter() + .map(|f| f.ident.as_ref().unwrap()) + .collect(), + _ => panic!("only unit variants or named fields are supported in storage enums"), + } + } + + /// Returns the field types of a storage enum variant. + fn types(&self) -> Vec<&Type> { + match self { + Self::Unit => vec![], + Self::Named(fields) => fields.named.iter().map(|f| &f.ty).collect(), + _ => panic!("only unit variants or named fields are supported in storage enums"), + } + } +} + +trait VariantExt { + fn storage_params(&self) -> TokenStream; + fn storage_key(&self, enum_name: &Ident) -> TokenStream; +} + +impl VariantExt for Variant { + /// Returns the parameter list for a storage enum variant. + fn storage_params(&self) -> TokenStream { + let (field_names, field_types) = (self.fields.names(), self.fields.types()); + + if field_names.is_empty() { + quote! { env: &soroban_sdk::Env } + } else { + quote! { env: &soroban_sdk::Env, #(#field_names: #field_types),* } + } + } + + /// Returns the key for a storage enum variant. + fn storage_key(&self, enum_name: &Ident) -> TokenStream { + let field_names = self.fields.names(); + let variant_ident = &self.ident; + + if field_names.is_empty() { + quote! { #enum_name::#variant_ident } + } else { + let field_names = field_names.iter().map(|name| quote! { #name }); + quote! { #enum_name::#variant_ident(#(#field_names),*) } + } + } +} + +impl Value { + /// Returns the getter, setter, and remover functions for a storage enum variant. + fn storage_fns( + &self, + enum_name: &Ident, + storage_type: &StorageType, + variant: &Variant, + ) -> TokenStream { + let (getter_name, setter_name, remover_name, try_getter_name) = + self.fn_names(&variant.ident); + let params = variant.storage_params(); + let storage_key = variant.storage_key(enum_name); + + match self { + Self::Status => { + let status_fns = self.status_fns( + storage_type, + &getter_name, + &setter_name, + &remover_name, + ¶ms, + &storage_key, + ); + + quote! { #status_fns } + } + Self::Type(value_type) => { + let value_fns = self.value_fns( + storage_type, + &getter_name, + &setter_name, + &remover_name, + &try_getter_name, + ¶ms, + &storage_key, + value_type, + ); + + quote! { #value_fns } + } + } + } + + fn status_fns( + &self, + storage_type: &StorageType, + getter_name: &Ident, + setter_name: &Ident, + remover_name: &Ident, + params: &TokenStream, + storage_key: &TokenStream, + ) -> TokenStream { + let storage_method = storage_type.storage_method(); + let ttl_fn = storage_type.ttl_fn("e! { key }); + + quote! { + pub fn #getter_name(#params) -> bool { + let key = #storage_key; + let value = #storage_method.has(&key); + + #ttl_fn + + value + } + + pub fn #setter_name(#params) { + let key = #storage_key; + #storage_method.set(&key, &()); + + #ttl_fn + } + + pub fn #remover_name(#params) { + let key = #storage_key; + #storage_method.remove(&key); + } + } + } + + fn value_fns( + &self, + storage_type: &StorageType, + getter_name: &Ident, + setter_name: &Ident, + remover_name: &Ident, + try_getter_name: &Ident, + params: &TokenStream, + storage_key: &TokenStream, + value_type: &Type, + ) -> TokenStream { + let storage_method = storage_type.storage_method(); + let ttl_fn = storage_type.ttl_fn("e! { key }); + + quote! { + pub fn #getter_name(#params) -> #value_type { + let key = #storage_key; + let value = #storage_method + .get::<_, #value_type>(&key) + .unwrap(); + + #ttl_fn + + value + } + + pub fn #try_getter_name(#params) -> Option<#value_type> { + let key = #storage_key; + let value = #storage_method.get::<_, #value_type>(&key); + + if value.is_some() { + #ttl_fn + } + + value + } + + pub fn #setter_name(#params, value: &#value_type) { + let key = #storage_key; + #storage_method.set(&key, value); + + #ttl_fn + } + + pub fn #remover_name(#params) { + let key = #storage_key; + #storage_method.remove(&key); + } + } + } + + /// Returns the getter, setter, and remover names for a storage enum variant. + fn fn_names(&self, variant_ident: &Ident) -> (Ident, Ident, Ident, Ident) { + let ident = variant_ident.to_string().to_snake_case(); + match self { + Self::Status => ( + format_ident!("is_{}", ident), + format_ident!("set_{}_status", ident), + format_ident!("remove_{}_status", ident), + format_ident!("_"), + ), + Self::Type(_) => ( + format_ident!("{}", ident), + format_ident!("set_{}", ident), + format_ident!("remove_{}", ident), + format_ident!("try_{}", ident), + ), + } + } } #[derive(Debug)] @@ -21,23 +253,41 @@ enum StorageType { Temporary, } +impl TryFrom<&[Attribute]> for StorageType { + type Error = String; + + fn try_from(attrs: &[Attribute]) -> Result { + attrs + .iter() + .flat_map(|attr| match attr { + _ if attr.path().is_ident("instance") => Some(Self::Instance), + _ if attr.path().is_ident("persistent") => Some(Self::Persistent), + _ if attr.path().is_ident("temporary") => Some(Self::Temporary), + _ => None, + }) + .exactly_one() + .map_err(|_| { + "storage type must be specified exactly once as 'instance', 'persistent', or 'temporary'" + .to_string() + }) + } +} + impl StorageType { - pub fn storage_method(&self) -> TokenStream { + fn storage_method(&self) -> TokenStream { match self { - Self::Instance => quote! { instance }, - Self::Persistent => quote! { persistent }, - Self::Temporary => quote! { temporary }, + Self::Persistent => quote! { env.storage().persistent() }, + Self::Instance => quote! { env.storage().instance() }, + Self::Temporary => quote! { env.storage().temporary() }, } } - pub fn ttl_method(&self) -> TokenStream { + fn ttl_fn(&self, ttl_fn_key: &TokenStream) -> TokenStream { match self { - Self::Persistent => quote! { - stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); - }, - Self::Instance => quote! { - stellar_axelar_std::ttl::extend_instance_ttl(env); - }, + Self::Persistent => { + quote! { stellar_axelar_std::ttl::extend_persistent_ttl(env, &#ttl_fn_key); } + } + Self::Instance => quote! { stellar_axelar_std::ttl::extend_instance_ttl(env); }, Self::Temporary => quote! {}, } } @@ -53,29 +303,23 @@ pub fn contract_storage(input: &DeriveInput) -> TokenStream { let transformed_variants: Vec<_> = variants.iter().map(transform_variant).collect(); - let public_fns: Vec<_> = variants + let fns: Vec<_> = variants .iter() .map(|variant| { - public_storage_fns( - name, - variant, - &StorageAttributes { - storage_type: storage_type(&variant.attrs), - value: value(&variant.attrs), - }, - ) + let storage_type = StorageType::try_from(variant.attrs.as_slice()).unwrap(); + let value = Value::try_from(variant.attrs.as_slice()).unwrap(); + + value.storage_fns(name, &storage_type, variant) }) .collect(); let contract_storage = quote! { - #[doc = "\n* Storage Enum\n"] #[contracttype] enum #name { #(#transformed_variants,)* } - #[doc = "\n* Public Functions\n"] - #(#public_fns)* + #(#fns)* }; let contract_storage_tests = contract_storage_tests(name, input); @@ -83,7 +327,6 @@ pub fn contract_storage(input: &DeriveInput) -> TokenStream { quote! { #contract_storage - #[doc = "\n* Contract Storage Tests\n"] #contract_storage_tests } } @@ -119,207 +362,18 @@ fn transform_variant(variant: &Variant) -> TokenStream { match &variant.fields { Fields::Unit => { - quote! { - #variant_name - } + quote! { #variant_name } } Fields::Named(FieldsNamed { named, .. }) => { let types = named.iter().map(|f| &f.ty); - quote! { - #variant_name(#(#types),*) - } + quote! { #variant_name(#(#types),*) } } _ => panic!("only unit variants or named fields are supported in storage enums"), } } -/// Returns the storage type of a storage enum variant. -fn storage_type(attrs: &[Attribute]) -> StorageType { - attrs.iter().flat_map(|attr| match attr { - _ if attr.path().is_ident("instance") => Some(StorageType::Instance), - _ if attr.path().is_ident("persistent") => Some(StorageType::Persistent), - _ if attr.path().is_ident("temporary") => Some(StorageType::Temporary), - _ => None}) - .exactly_one() - .expect("storage type must be specified exactly once as 'instance', 'persistent', or 'temporary'") -} - -/// Returns the status xor value type of a storage enum variant. -fn value(attrs: &[Attribute]) -> Value { - let has_status = attrs.iter().any(|attr| attr.path().is_ident("status")); - let value_attrs: Vec<_> = attrs - .iter() - .filter(|attr| attr.path().is_ident("value")) - .collect(); - - match (has_status, !value_attrs.is_empty()) { - (true, false) => Value::Status, - (false, true) => { - let attr = value_attrs[0]; - if let Meta::List(list) = &attr.meta { - Value::Type( - list.parse_args::() - .expect("failed to parse value type"), - ) - } else { - panic!("value attribute must contain a type parameter: #[value(Type)]"); - } - } - (false, false) => panic!("missing required attribute: either #[status] xor #[value(Type)]"), - _ => panic!("a storage key cannot have both #[status] and #[value] attributes"), - } -} - -/// Generates the public module-level storage functions. -fn public_storage_fns( - enum_name: &Ident, - variant: &Variant, - StorageAttributes { - storage_type, - value, - }: &StorageAttributes, -) -> TokenStream { - let variant_ident = &variant.ident; - - let (field_names, field_types) = fields_data(&variant.fields); - - let storage_method = storage_type.storage_method(); - let ttl_fn = storage_type.ttl_method(); - - let key = if field_names.is_empty() { - quote! { #enum_name::#variant_ident } - } else { - quote! { #enum_name::#variant_ident(#(#field_names),*) } - }; - let param_list = if field_names.is_empty() { - quote! { env: &soroban_sdk::Env } - } else { - quote! { env: &soroban_sdk::Env, #(#field_names: #field_types),* } - }; - - match &value { - Value::Status => value_status_fns(variant, ¶m_list, &storage_method, key), - Value::Type(value_type) => value_type_fns( - variant, - ¶m_list, - &storage_method, - key, - value_type, - &ttl_fn, - ), - } -} - -fn value_status_fns( - variant: &Variant, - param_list: &TokenStream, - storage_method: &TokenStream, - key: TokenStream, -) -> TokenStream { - let (getter_name, setter_name, remover_name) = fn_names(variant, true); - - quote! { - #[doc = " Status Getter"] - pub fn #getter_name(#param_list) -> bool { - env.storage() - .#storage_method() - .has(&#key) - } - - #[doc = " Status Setter"] - pub fn #setter_name(#param_list) { - env.storage() - .#storage_method() - .set(&#key, &()); - } - - #[doc = " Status Remover"] - pub fn #remover_name(#param_list) { - env.storage() - .#storage_method() - .remove(&#key); - } - } -} - -fn value_type_fns( - variant: &Variant, - param_list: &TokenStream, - storage_method: &TokenStream, - key: TokenStream, - value_type: &Type, - ttl_fn: &TokenStream, -) -> TokenStream { - let (getter_name, setter_name, remover_name) = fn_names(variant, false); - - quote! { - #[doc = " Value Type Getter"] - pub fn #getter_name(#param_list) -> Option<#value_type> { - let key = #key; - let value = env.storage() - .#storage_method() - .get::<_, #value_type>(&key); - - if value.is_some() { - #ttl_fn - } - - value - } - - #[doc = " Value Type Setter"] - pub fn #setter_name(#param_list, value: &#value_type) { - let key = #key; - - env.storage() - .#storage_method() - .set(&key, value); - - #ttl_fn - } - - #[doc = " Value Type Remover"] - pub fn #remover_name(#param_list) { - env.storage() - .#storage_method() - .remove(&#key); - } - } -} - -/// Returns the field names and types of a storage enum variant. -fn fields_data(fields: &Fields) -> (Vec<&Option>, Vec<&Type>) { - match fields { - Fields::Unit => (vec![], vec![]), - Fields::Named(fields) => { - let names = fields.named.iter().map(|f| &f.ident).collect(); - let types = fields.named.iter().map(|f| &f.ty).collect(); - (names, types) - } - _ => panic!("only unit variants or named fields are supported in storage enums"), - } -} - -fn fn_names(variant: &Variant, status: bool) -> (Ident, Ident, Ident) { - if status { - ( - format_ident!("is_{}", variant.ident.to_string().to_snake_case()), - format_ident!("set_{}_status", variant.ident.to_string().to_snake_case()), - format_ident!( - "remove_{}_status", - variant.ident.to_string().to_snake_case() - ), - ) - } else { - ( - format_ident!("{}", variant.ident.to_string().to_snake_case()), - format_ident!("set_{}", variant.ident.to_string().to_snake_case()), - format_ident!("remove_{}", variant.ident.to_string().to_snake_case()), - ) - } -} - -fn contract_storage_tests(enum_name: &Ident, input: &DeriveInput) -> TokenStream { +/// Generates the storage schema tests for a storage enum. +fn contract_storage_tests(enum_name: &Ident, enum_input: &DeriveInput) -> TokenStream { let test_module_name = format_ident!( "{}_storage_layout_tests", enum_name.to_string().to_snake_case() @@ -330,25 +384,35 @@ fn contract_storage_tests(enum_name: &Ident, input: &DeriveInput) -> TokenStream enum_name.to_string().to_snake_case() ); + let enum_file: syn::File = syn::parse2(quote! { #enum_input }).unwrap(); + let formatted_enum = unparse(&enum_file) + .replace(" #[instance]", "\n #[instance]") + .replace(" #[persistent]", "\n #[persistent]") + .replace(" #[temporary]", "\n #[temporary]"); + quote! { #[cfg(test)] mod #test_module_name { + use goldie; + #[test] fn #test_name() { - goldie::assert!(stringify!(#input)); + goldie::assert!(#formatted_enum); } } } } +/// Tests the storage schema generation for a storage enum. #[cfg(test)] mod tests { + use syn::{DeriveInput, Fields}; - use super::*; + use crate::storage::contract_storage; #[test] fn storage_schema_generation_succeeds() { - let input: DeriveInput = syn::parse_quote! { + let enum_input: DeriveInput = syn::parse_quote! { enum DataKey { #[instance] #[value(u32)] @@ -380,10 +444,12 @@ mod tests { } }; - let generated = contract_storage(&input); - let file: syn::File = syn::parse2(generated).unwrap(); - let formatted = prettyplease::unparse(&file); - goldie::assert!(formatted); + let storage_module = contract_storage(&enum_input); + let storage_module_file: syn::File = syn::parse2(storage_module).unwrap(); + let formatted_storage_module = prettyplease::unparse(&storage_module_file) + .replace("pub fn ", "\npub fn ") + .replace("#[cfg(test)]", "\n#[cfg(test)]"); + goldie::assert!(formatted_storage_module); } #[test] @@ -428,12 +494,11 @@ mod tests { } #[test] - #[should_panic(expected = "value attribute must contain a type parameter: #[value(Type)]")] - fn missing_value_fails() { + #[should_panic(expected = "exactly one of #[status] and #[value(Type)] must be provided")] + fn missing_value_and_status_attribute_fails() { let input: DeriveInput = syn::parse_quote! { enum InvalidEnum { #[instance] - #[value] Counter, } }; @@ -442,30 +507,29 @@ mod tests { } #[test] - #[should_panic(expected = "missing required attribute: either #[status] xor #[value(Type)]")] - fn missing_value_attribute_fails() { - let input: DeriveInput = syn::parse_quote! { - enum InvalidEnum { - #[instance] - Counter, - } - }; + #[should_panic(expected = "only unit variants or named fields are supported in storage enums")] + fn fields_names_tuple_variant_fails() { + use super::FieldsExt; + let fields = Fields::Unnamed(syn::parse_quote! { + (String, u32) + }); - contract_storage(&input); + fields.names(); } #[test] #[should_panic(expected = "only unit variants or named fields are supported in storage enums")] - fn fields_data_tuple_variant_fails() { + fn fields_types_tuple_variant_fails() { + use super::FieldsExt; let fields = Fields::Unnamed(syn::parse_quote! { (String, u32) }); - fields_data(&fields); + fields.types(); } #[test] - #[should_panic(expected = "a storage key cannot have both #[status] and #[value] attributes")] + #[should_panic(expected = "exactly one of #[status] and #[value(Type)] must be provided")] fn status_and_value_fails() { let input: DeriveInput = syn::parse_quote! { enum InvalidEnum { @@ -478,4 +542,32 @@ mod tests { contract_storage(&input); } + + #[test] + #[should_panic(expected = "failed to parse value type")] + fn invalid_value_type_fails() { + let input: DeriveInput = syn::parse_quote! { + enum InvalidEnum { + #[instance] + #[value(!@$Type)] + InvalidKey, + } + }; + + contract_storage(&input); + } + + #[test] + #[should_panic(expected = "value attribute must contain a type parameter: #[value(Type)]")] + fn value_without_type_fails() { + let input: DeriveInput = syn::parse_quote! { + enum InvalidEnum { + #[instance] + #[value] + InvalidKey, + } + }; + + contract_storage(&input); + } } diff --git a/packages/stellar-axelar-std-derive/src/testdata/storage_schema_generation_succeeds.golden b/packages/stellar-axelar-std-derive/src/testdata/storage_schema_generation_succeeds.golden index 2047b2f0..32fa267e 100644 --- a/packages/stellar-axelar-std-derive/src/testdata/storage_schema_generation_succeeds.golden +++ b/packages/stellar-axelar-std-derive/src/testdata/storage_schema_generation_succeeds.golden @@ -1,6 +1,3 @@ -/** -* Storage Enum -*/ #[contracttype] enum DataKey { Counter, @@ -11,11 +8,15 @@ enum DataKey { Initialized, Paused, } -/** -* Public Functions -*/ -/// Value Type Getter -pub fn counter(env: &soroban_sdk::Env) -> Option { + +pub fn counter(env: &soroban_sdk::Env) -> u32 { + let key = DataKey::Counter; + let value = env.storage().instance().get::<_, u32>(&key).unwrap(); + stellar_axelar_std::ttl::extend_instance_ttl(env); + value +} + +pub fn try_counter(env: &soroban_sdk::Env) -> Option { let key = DataKey::Counter; let value = env.storage().instance().get::<_, u32>(&key); if value.is_some() { @@ -23,18 +24,26 @@ pub fn counter(env: &soroban_sdk::Env) -> Option { } value } -/// Value Type Setter + pub fn set_counter(env: &soroban_sdk::Env, value: &u32) { let key = DataKey::Counter; env.storage().instance().set(&key, value); stellar_axelar_std::ttl::extend_instance_ttl(env); } -/// Value Type Remover + pub fn remove_counter(env: &soroban_sdk::Env) { - env.storage().instance().remove(&DataKey::Counter); + let key = DataKey::Counter; + env.storage().instance().remove(&key); } -/// Value Type Getter -pub fn message(env: &soroban_sdk::Env, sender: Address) -> Option { + +pub fn message(env: &soroban_sdk::Env, sender: Address) -> String { + let key = DataKey::Message(sender); + let value = env.storage().persistent().get::<_, String>(&key).unwrap(); + stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); + value +} + +pub fn try_message(env: &soroban_sdk::Env, sender: Address) -> Option { let key = DataKey::Message(sender); let value = env.storage().persistent().get::<_, String>(&key); if value.is_some() { @@ -42,34 +51,49 @@ pub fn message(env: &soroban_sdk::Env, sender: Address) -> Option { } value } -/// Value Type Setter + pub fn set_message(env: &soroban_sdk::Env, sender: Address, value: &String) { let key = DataKey::Message(sender); env.storage().persistent().set(&key, value); stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); } -/// Value Type Remover + pub fn remove_message(env: &soroban_sdk::Env, sender: Address) { - env.storage().persistent().remove(&DataKey::Message(sender)); + let key = DataKey::Message(sender); + env.storage().persistent().remove(&key); } -/// Value Type Getter -pub fn last_caller(env: &soroban_sdk::Env, timestamp: u64) -> Option
{ + +pub fn last_caller(env: &soroban_sdk::Env, timestamp: u64) -> Address { + let key = DataKey::LastCaller(timestamp); + let value = env.storage().temporary().get::<_, Address>(&key).unwrap(); + value +} + +pub fn try_last_caller(env: &soroban_sdk::Env, timestamp: u64) -> Option
{ let key = DataKey::LastCaller(timestamp); let value = env.storage().temporary().get::<_, Address>(&key); if value.is_some() {} value } -/// Value Type Setter + pub fn set_last_caller(env: &soroban_sdk::Env, timestamp: u64, value: &Address) { let key = DataKey::LastCaller(timestamp); env.storage().temporary().set(&key, value); } -/// Value Type Remover + pub fn remove_last_caller(env: &soroban_sdk::Env, timestamp: u64) { - env.storage().temporary().remove(&DataKey::LastCaller(timestamp)); + let key = DataKey::LastCaller(timestamp); + env.storage().temporary().remove(&key); +} + +pub fn flag(env: &soroban_sdk::Env, key: String, owner: Address) -> bool { + let key = DataKey::Flag(key, owner); + let value = env.storage().persistent().get::<_, bool>(&key).unwrap(); + stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); + value } -/// Value Type Getter -pub fn flag(env: &soroban_sdk::Env, key: String, owner: Address) -> Option { + +pub fn try_flag(env: &soroban_sdk::Env, key: String, owner: Address) -> Option { let key = DataKey::Flag(key, owner); let value = env.storage().persistent().get::<_, bool>(&key); if value.is_some() { @@ -77,18 +101,26 @@ pub fn flag(env: &soroban_sdk::Env, key: String, owner: Address) -> Option } value } -/// Value Type Setter + pub fn set_flag(env: &soroban_sdk::Env, key: String, owner: Address, value: &bool) { let key = DataKey::Flag(key, owner); env.storage().persistent().set(&key, value); stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); } -/// Value Type Remover + pub fn remove_flag(env: &soroban_sdk::Env, key: String, owner: Address) { - env.storage().persistent().remove(&DataKey::Flag(key, owner)); + let key = DataKey::Flag(key, owner); + env.storage().persistent().remove(&key); +} + +pub fn optional_message(env: &soroban_sdk::Env, id: u32) -> Option { + let key = DataKey::OptionalMessage(id); + let value = env.storage().persistent().get::<_, Option>(&key).unwrap(); + stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); + value } -/// Value Type Getter -pub fn optional_message(env: &soroban_sdk::Env, id: u32) -> Option> { + +pub fn try_optional_message(env: &soroban_sdk::Env, id: u32) -> Option> { let key = DataKey::OptionalMessage(id); let value = env.storage().persistent().get::<_, Option>(&key); if value.is_some() { @@ -96,54 +128,61 @@ pub fn optional_message(env: &soroban_sdk::Env, id: u32) -> Option) { let key = DataKey::OptionalMessage(id); env.storage().persistent().set(&key, value); stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); } -/// Value Type Remover + pub fn remove_optional_message(env: &soroban_sdk::Env, id: u32) { - env.storage().persistent().remove(&DataKey::OptionalMessage(id)); + let key = DataKey::OptionalMessage(id); + env.storage().persistent().remove(&key); } -/// Status Getter + pub fn is_initialized(env: &soroban_sdk::Env) -> bool { - env.storage().instance().has(&DataKey::Initialized) + let key = DataKey::Initialized; + let value = env.storage().instance().has(&key); + stellar_axelar_std::ttl::extend_instance_ttl(env); + value } -/// Status Setter + pub fn set_initialized_status(env: &soroban_sdk::Env) { - env.storage().instance().set(&DataKey::Initialized, &()); + let key = DataKey::Initialized; + env.storage().instance().set(&key, &()); + stellar_axelar_std::ttl::extend_instance_ttl(env); } -/// Status Remover + pub fn remove_initialized_status(env: &soroban_sdk::Env) { - env.storage().instance().remove(&DataKey::Initialized); + let key = DataKey::Initialized; + env.storage().instance().remove(&key); } -/// Status Getter + pub fn is_paused(env: &soroban_sdk::Env) -> bool { - env.storage().persistent().has(&DataKey::Paused) + let key = DataKey::Paused; + let value = env.storage().persistent().has(&key); + stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); + value } -/// Status Setter + pub fn set_paused_status(env: &soroban_sdk::Env) { - env.storage().persistent().set(&DataKey::Paused, &()); + let key = DataKey::Paused; + env.storage().persistent().set(&key, &()); + stellar_axelar_std::ttl::extend_persistent_ttl(env, &key); } -/// Status Remover + pub fn remove_paused_status(env: &soroban_sdk::Env) { - env.storage().persistent().remove(&DataKey::Paused); + let key = DataKey::Paused; + env.storage().persistent().remove(&key); } -/** -* Contract Storage Tests -*/ + #[cfg(test)] mod data_key_storage_layout_tests { + use goldie; #[test] fn ensure_data_key_storage_schema_is_unchanged() { goldie::assert!( - stringify!(enum DataKey { #[instance] #[value(u32)] Counter, #[persistent] - #[value(String)] Message { sender : Address }, #[temporary] #[value(Address)] - LastCaller { timestamp : u64 }, #[persistent] #[value(bool)] Flag { key : - String, owner : Address }, #[persistent] #[value(Option < String >)] - OptionalMessage { id : u32 }, #[instance] #[status] Initialized, - #[persistent] #[status] Paused, }) + "enum DataKey {\n\n #[instance]\n #[value(u32)]\n Counter,\n\n #[persistent]\n #[value(String)]\n Message { sender: Address },\n\n #[temporary]\n #[value(Address)]\n LastCaller { timestamp: u64 },\n\n #[persistent]\n #[value(bool)]\n Flag { key: String, owner: Address },\n\n #[persistent]\n #[value(Option)]\n OptionalMessage { id: u32 },\n\n #[instance]\n #[status]\n Initialized,\n\n #[persistent]\n #[status]\n Paused,\n}\n" ); } } diff --git a/packages/stellar-axelar-std/src/tests/storage.rs b/packages/stellar-axelar-std/src/tests/storage.rs index 73b43834..f08dac26 100644 --- a/packages/stellar-axelar-std/src/tests/storage.rs +++ b/packages/stellar-axelar-std/src/tests/storage.rs @@ -32,6 +32,10 @@ mod storage { #[persistent] #[value(Option)] OptionalMessage { id: u32 }, + + #[temporary] + #[status] + TempStatus { id: u32 }, } } @@ -40,7 +44,7 @@ impl Contract { pub const fn __constructor() {} pub fn increment_counter(env: &Env) -> u32 { - let current_counter = storage::counter(env).unwrap_or(0); + let current_counter = storage::try_counter(env).unwrap_or(0); let new_counter = current_counter + 1; storage::set_counter(env, &new_counter); new_counter @@ -51,7 +55,7 @@ impl Contract { } pub fn message(env: &Env, sender: Address) -> Option { - storage::message(env, sender) + storage::try_message(env, sender) } pub fn set_last_caller(env: &Env, timestamp: u64, caller: Address) { @@ -59,7 +63,7 @@ impl Contract { } pub fn last_caller(env: &Env, timestamp: u64) -> Option
{ - storage::last_caller(env, timestamp) + storage::try_last_caller(env, timestamp) } pub fn set_flag(env: &Env, key: String, owner: Address, value: bool) { @@ -67,7 +71,7 @@ impl Contract { } pub fn flag(env: &Env, key: String, owner: Address) -> Option { - storage::flag(env, key, owner) + storage::try_flag(env, key, owner) } pub fn set_optional_message(env: &Env, id: u32, message: Option) { @@ -75,7 +79,23 @@ impl Contract { } pub fn optional_message(env: &Env, id: u32) -> Option> { - storage::optional_message(env, id) + storage::try_optional_message(env, id) + } + + pub fn message_required(env: &Env, sender: Address) -> String { + storage::message(env, sender) + } + + pub fn flag_required(env: &Env, key: String, owner: Address) -> bool { + storage::flag(env, key, owner) + } + + pub fn set_temp_status(env: &Env, id: u32) { + storage::set_temp_status_status(env, id); + } + + pub fn is_temp_status(env: &Env, id: u32) -> bool { + storage::is_temp_status(env, id) } } @@ -232,3 +252,41 @@ fn optional_value_storage_succeeds() { client.set_optional_message(&id, &None); assert_eq!(client.optional_message(&id), None); } + +#[test] +#[should_panic(expected = "called `Option::unwrap()` on a `None` value")] +fn required_getter_fails_when_not_set() { + let env = Env::default(); + let contract_id = env.register(Contract, ()); + let client = ContractClient::new(&env, &contract_id); + + let sender = Address::generate(&env); + client.message_required(&sender); +} + +#[test] +fn required_getter_succeeds_when_set() { + let env = Env::default(); + let contract_id = env.register(Contract, ()); + let client = ContractClient::new(&env, &contract_id); + + let sender = Address::generate(&env); + let message = String::from_str(&env, "Required message"); + + client.set_message(&sender, &message); + assert_eq!(client.message_required(&sender), message); +} + +#[test] +fn temporary_status_storage_succeeds() { + let env = Env::default(); + let contract_id = env.register(Contract, ()); + let client = ContractClient::new(&env, &contract_id); + + let id = 1u32; + + assert!(!client.is_temp_status(&id)); + + client.set_temp_status(&id); + assert!(client.is_temp_status(&id)); +} diff --git a/packages/stellar-axelar-std/src/tests/testdata/ensure_data_key_storage_schema_is_unchanged.golden b/packages/stellar-axelar-std/src/tests/testdata/ensure_data_key_storage_schema_is_unchanged.golden index 5b304bee..782d2e1a 100644 --- a/packages/stellar-axelar-std/src/tests/testdata/ensure_data_key_storage_schema_is_unchanged.golden +++ b/packages/stellar-axelar-std/src/tests/testdata/ensure_data_key_storage_schema_is_unchanged.golden @@ -1,8 +1,26 @@ -enum DataKey -{ - #[instance] #[value(u32)] Counter, #[persistent] #[value(String)] Message - { sender : Address }, #[temporary] #[value(Address)] LastCaller - { timestamp : u64 }, #[persistent] #[value(bool)] Flag - { key : String, owner : Address }, #[persistent] #[value(Option)] - OptionalMessage { id : u32 }, -} \ No newline at end of file +enum DataKey { + + #[instance] + #[value(u32)] + Counter, + + #[persistent] + #[value(String)] + Message { sender: Address }, + + #[temporary] + #[value(Address)] + LastCaller { timestamp: u64 }, + + #[persistent] + #[value(bool)] + Flag { key: String, owner: Address }, + + #[persistent] + #[value(Option)] + OptionalMessage { id: u32 }, + + #[temporary] + #[status] + TempStatus { id: u32 }, +} From aa1f16704d1d2841b0e382443d8b1b42db341f3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Noah=20B=2E=20=F0=9F=A5=82?= <15343884+nbayindirli@users.noreply.github.com> Date: Thu, 6 Feb 2025 15:09:15 -0800 Subject: [PATCH 4/6] refactor(axelar-operators): use contractstorage (#244) --- .../stellar-axelar-operators/src/contract.rs | 22 ++++++------------- contracts/stellar-axelar-operators/src/lib.rs | 2 +- .../stellar-axelar-operators/src/storage.rs | 10 +++++++++ .../src/storage_types.rs | 7 ------ ...ata_key_storage_schema_is_unchanged.golden | 7 ++++++ 5 files changed, 25 insertions(+), 23 deletions(-) create mode 100644 contracts/stellar-axelar-operators/src/storage.rs delete mode 100644 contracts/stellar-axelar-operators/src/storage_types.rs create mode 100644 contracts/stellar-axelar-operators/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden diff --git a/contracts/stellar-axelar-operators/src/contract.rs b/contracts/stellar-axelar-operators/src/contract.rs index b04a8ed7..069e5020 100644 --- a/contracts/stellar-axelar-operators/src/contract.rs +++ b/contracts/stellar-axelar-operators/src/contract.rs @@ -6,7 +6,7 @@ use stellar_axelar_std::{ensure, interfaces, Ownable, Upgradable}; use crate::error::ContractError; use crate::event::{OperatorAddedEvent, OperatorRemovedEvent}; use crate::interface::AxelarOperatorsInterface; -use crate::storage_types::DataKey; +use crate::storage; #[contract] #[derive(Ownable, Upgradable)] @@ -22,22 +22,18 @@ impl AxelarOperators { #[contractimpl] impl AxelarOperatorsInterface for AxelarOperators { fn is_operator(env: Env, account: Address) -> bool { - let key = DataKey::Operators(account); - - env.storage().instance().has(&key) + storage::is_operators(&env, account) } fn add_operator(env: Env, account: Address) -> Result<(), ContractError> { Self::owner(&env).require_auth(); - let key = DataKey::Operators(account.clone()); - ensure!( - !env.storage().instance().has(&key), + !storage::is_operators(&env, account.clone()), ContractError::OperatorAlreadyAdded ); - env.storage().instance().set(&key, &true); + storage::set_operators_status(&env, account.clone()); extend_instance_ttl(&env); @@ -49,14 +45,12 @@ impl AxelarOperatorsInterface for AxelarOperators { fn remove_operator(env: Env, account: Address) -> Result<(), ContractError> { Self::owner(&env).require_auth(); - let key = DataKey::Operators(account.clone()); - ensure!( - env.storage().instance().has(&key), + storage::is_operators(&env, account.clone()), ContractError::NotAnOperator ); - env.storage().instance().remove(&key); + storage::remove_operators_status(&env, account.clone()); OperatorRemovedEvent { operator: account }.emit(&env); @@ -72,10 +66,8 @@ impl AxelarOperatorsInterface for AxelarOperators { ) -> Result { operator.require_auth(); - let key = DataKey::Operators(operator); - ensure!( - env.storage().instance().has(&key), + storage::is_operators(&env, operator), ContractError::NotAnOperator ); diff --git a/contracts/stellar-axelar-operators/src/lib.rs b/contracts/stellar-axelar-operators/src/lib.rs index d6994a1b..0714ee85 100644 --- a/contracts/stellar-axelar-operators/src/lib.rs +++ b/contracts/stellar-axelar-operators/src/lib.rs @@ -15,7 +15,7 @@ cfg_if::cfg_if! { pub use interface::{AxelarOperatorsClient, AxelarOperatorsInterface}; } else { pub mod event; - mod storage_types; + mod storage; mod contract; pub use contract::{AxelarOperators, AxelarOperatorsClient}; diff --git a/contracts/stellar-axelar-operators/src/storage.rs b/contracts/stellar-axelar-operators/src/storage.rs new file mode 100644 index 00000000..18fcadd0 --- /dev/null +++ b/contracts/stellar-axelar-operators/src/storage.rs @@ -0,0 +1,10 @@ +use soroban_sdk::{contracttype, Address}; +use stellar_axelar_std::contractstorage; + +#[contractstorage] +#[derive(Clone, Debug)] +pub enum DataKey { + #[instance] + #[status] + Operators { account: Address }, +} diff --git a/contracts/stellar-axelar-operators/src/storage_types.rs b/contracts/stellar-axelar-operators/src/storage_types.rs deleted file mode 100644 index be67dc38..00000000 --- a/contracts/stellar-axelar-operators/src/storage_types.rs +++ /dev/null @@ -1,7 +0,0 @@ -use soroban_sdk::{contracttype, Address}; - -#[contracttype] -#[derive(Clone, Debug)] -pub enum DataKey { - Operators(Address), -} diff --git a/contracts/stellar-axelar-operators/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden b/contracts/stellar-axelar-operators/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden new file mode 100644 index 00000000..14cb3af3 --- /dev/null +++ b/contracts/stellar-axelar-operators/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden @@ -0,0 +1,7 @@ +#[derive(Clone, Debug)] +pub enum DataKey { + + #[instance] + #[status] + Operators { account: Address }, +} From a1ac57ac6fe60befee74020ce38d17a3e9e7249d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Noah=20B=2E=20=F0=9F=A5=82?= <15343884+nbayindirli@users.noreply.github.com> Date: Thu, 6 Feb 2025 15:57:22 -0800 Subject: [PATCH 5/6] refactor(axelar-gateway): use contractstorage (#243) --- contracts/stellar-axelar-gateway/src/auth.rs | 99 ++++--------------- .../stellar-axelar-gateway/src/contract.rs | 45 ++++----- contracts/stellar-axelar-gateway/src/lib.rs | 2 +- .../stellar-axelar-gateway/src/storage.rs | 55 +++++++++++ .../src/storage_types.rs | 31 ------ ...ata_key_storage_schema_is_unchanged.golden | 35 +++++++ 6 files changed, 128 insertions(+), 139 deletions(-) create mode 100644 contracts/stellar-axelar-gateway/src/storage.rs delete mode 100644 contracts/stellar-axelar-gateway/src/storage_types.rs create mode 100644 contracts/stellar-axelar-gateway/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden diff --git a/contracts/stellar-axelar-gateway/src/auth.rs b/contracts/stellar-axelar-gateway/src/auth.rs index 487a28b5..ca029cda 100644 --- a/contracts/stellar-axelar-gateway/src/auth.rs +++ b/contracts/stellar-axelar-gateway/src/auth.rs @@ -4,7 +4,7 @@ use stellar_axelar_std::events::Event; use crate::error::ContractError; use crate::event::SignersRotatedEvent; -use crate::storage_types::DataKey; +use crate::storage; use crate::types::{Proof, ProofSignature, ProofSigner, WeightedSigner, WeightedSigners}; pub fn initialize_auth( @@ -14,20 +14,10 @@ pub fn initialize_auth( previous_signer_retention: u64, initial_signers: Vec, ) -> Result<(), ContractError> { - env.storage().instance().set(&DataKey::Epoch, &0_u64); - - env.storage().instance().set( - &DataKey::PreviousSignerRetention, - &previous_signer_retention, - ); - - env.storage() - .instance() - .set(&DataKey::DomainSeparator, &domain_separator); - - env.storage() - .instance() - .set(&DataKey::MinimumRotationDelay, &minimum_rotation_delay); + storage::set_epoch(&env, &0_u64); + storage::set_previous_signer_retention(&env, &previous_signer_retention); + storage::set_domain_separator(&env, &domain_separator); + storage::set_minimum_rotation_delay(&env, &minimum_rotation_delay); ensure!(!initial_signers.is_empty(), ContractError::EmptySigners); @@ -38,27 +28,6 @@ pub fn initialize_auth( Ok(()) } -pub fn domain_separator(env: &Env) -> BytesN<32> { - env.storage() - .instance() - .get(&DataKey::DomainSeparator) - .expect("domain_separator not found") -} - -pub fn minimum_rotation_delay(env: &Env) -> u64 { - env.storage() - .instance() - .get(&DataKey::MinimumRotationDelay) - .expect("minimum_rotation_delay not found") -} - -pub fn previous_signers_retention(env: &Env) -> u64 { - env.storage() - .instance() - .get(&DataKey::PreviousSignerRetention) - .expect("previous_signers_retention not found") -} - pub fn validate_proof( env: &Env, data_hash: &BytesN<32>, @@ -68,14 +37,15 @@ pub fn validate_proof( let signers_hash = signers_set.hash(env); - let signers_epoch = epoch_by_signers_hash(env, signers_hash.clone())?; + let signers_epoch = storage::try_epoch_by_signers_hash(env, signers_hash.clone()) + .ok_or(ContractError::InvalidSignersHash)?; - let current_epoch = epoch(env); + let current_epoch = storage::epoch(env); let is_latest_signers: bool = signers_epoch == current_epoch; ensure!( - current_epoch - signers_epoch <= previous_signers_retention(env), + current_epoch - signers_epoch <= storage::previous_signer_retention(env), ContractError::OutdatedSigners ); @@ -100,23 +70,18 @@ pub fn rotate_signers( let new_signers_hash = new_signers.hash(env); - let new_epoch: u64 = epoch(env) + 1; + let new_epoch = storage::epoch(env) + 1; - env.storage().instance().set(&DataKey::Epoch, &new_epoch); + storage::set_epoch(env, &new_epoch); - env.storage() - .persistent() - .set(&DataKey::SignersHashByEpoch(new_epoch), &new_signers_hash); + storage::set_signers_hash_by_epoch(env, new_epoch, &new_signers_hash); ensure!( - epoch_by_signers_hash(env, new_signers_hash.clone()).is_err(), + storage::try_epoch_by_signers_hash(env, new_signers_hash.clone()).is_none(), ContractError::DuplicateSigners ); - env.storage().persistent().set( - &DataKey::EpochBySignersHash(new_signers_hash.clone()), - &new_epoch, - ); + storage::set_epoch_by_signers_hash(env, new_signers_hash.clone(), &new_epoch); SignersRotatedEvent { epoch: new_epoch, @@ -128,29 +93,8 @@ pub fn rotate_signers( Ok(()) } -pub fn epoch(env: &Env) -> u64 { - env.storage() - .instance() - .get(&DataKey::Epoch) - .expect("epoch not found") -} - -pub fn epoch_by_signers_hash(env: &Env, signers_hash: BytesN<32>) -> Result { - env.storage() - .persistent() - .get(&DataKey::EpochBySignersHash(signers_hash)) - .ok_or(ContractError::InvalidSignersHash) -} - -pub fn signers_hash_by_epoch(env: &Env, epoch: u64) -> Result, ContractError> { - env.storage() - .persistent() - .get(&DataKey::SignersHashByEpoch(epoch)) - .ok_or(ContractError::InvalidEpoch) -} - fn message_hash_to_sign(env: &Env, signers_hash: BytesN<32>, data_hash: &BytesN<32>) -> BytesN<32> { - let mut msg: Bytes = domain_separator(env).into(); + let mut msg: Bytes = storage::domain_separator(env).into(); msg.extend_from_array(&signers_hash.to_array()); msg.extend_from_array(&data_hash.to_array()); @@ -158,24 +102,17 @@ fn message_hash_to_sign(env: &Env, signers_hash: BytesN<32>, data_hash: &BytesN< } fn update_rotation_timestamp(env: &Env, enforce_rotation_delay: bool) -> Result<(), ContractError> { - let last_rotation_timestamp: u64 = env - .storage() - .instance() - .get(&DataKey::LastRotationTimestamp) - .unwrap_or(0); - let current_timestamp = env.ledger().timestamp(); if enforce_rotation_delay { ensure!( - current_timestamp - last_rotation_timestamp >= minimum_rotation_delay(env), + current_timestamp - storage::last_rotation_timestamp(env) + >= storage::minimum_rotation_delay(env), ContractError::InsufficientRotationDelay ); } - env.storage() - .instance() - .set(&DataKey::LastRotationTimestamp, ¤t_timestamp); + storage::set_last_rotation_timestamp(env, ¤t_timestamp); Ok(()) } diff --git a/contracts/stellar-axelar-gateway/src/contract.rs b/contracts/stellar-axelar-gateway/src/contract.rs index e606d8ee..e321c267 100644 --- a/contracts/stellar-axelar-gateway/src/contract.rs +++ b/contracts/stellar-axelar-gateway/src/contract.rs @@ -6,13 +6,13 @@ use stellar_axelar_std::{ ensure, interfaces, when_not_paused, Operatable, Ownable, Pausable, Upgradable, }; -use crate::auth; use crate::error::ContractError; use crate::event::{ContractCalledEvent, MessageApprovedEvent, MessageExecutedEvent}; use crate::interface::AxelarGatewayInterface; use crate::messaging_interface::AxelarGatewayMessagingInterface; -use crate::storage_types::{DataKey, MessageApprovalKey, MessageApprovalValue}; +use crate::storage::{MessageApprovalKey, MessageApprovalValue}; use crate::types::{CommandType, Message, Proof, WeightedSigners}; +use crate::{auth, storage}; #[contract] #[derive(Operatable, Ownable, Pausable, Upgradable)] @@ -110,7 +110,8 @@ impl AxelarGatewayMessagingInterface for AxelarGateway { source_chain: source_chain.clone(), message_id: message_id.clone(), }; - let message_approval = Self::message_approval_by_key(&env, key.clone()); + let message_approval = storage::try_message_approval(&env, key.clone()) + .unwrap_or(MessageApprovalValue::NotApproved); let message = Message { source_chain, message_id, @@ -120,10 +121,7 @@ impl AxelarGatewayMessagingInterface for AxelarGateway { }; if message_approval == Self::message_approval_hash(&env, message.clone()) { - env.storage().persistent().set( - &DataKey::MessageApproval(key), - &MessageApprovalValue::Executed, - ); + storage::set_message_approval(&env, key, &MessageApprovalValue::Executed); MessageExecutedEvent { message }.emit(&env); @@ -137,15 +135,15 @@ impl AxelarGatewayMessagingInterface for AxelarGateway { #[contractimpl] impl AxelarGatewayInterface for AxelarGateway { fn domain_separator(env: &Env) -> BytesN<32> { - auth::domain_separator(env) + storage::domain_separator(env) } fn minimum_rotation_delay(env: &Env) -> u64 { - auth::minimum_rotation_delay(env) + storage::minimum_rotation_delay(env) } fn previous_signers_retention(env: &Env) -> u64 { - auth::previous_signers_retention(env) + storage::previous_signer_retention(env) } #[when_not_paused] @@ -164,19 +162,21 @@ impl AxelarGatewayInterface for AxelarGateway { ensure!(!messages.is_empty(), ContractError::EmptyMessages); for message in messages.into_iter() { - let key = MessageApprovalKey { + let message_approval_key = MessageApprovalKey { source_chain: message.source_chain.clone(), message_id: message.message_id.clone(), }; // Prevent replay if message is already approved/executed - let message_approval = Self::message_approval_by_key(env, key.clone()); + let message_approval = storage::try_message_approval(env, message_approval_key.clone()) + .unwrap_or(MessageApprovalValue::NotApproved); if message_approval != MessageApprovalValue::NotApproved { continue; } - env.storage().persistent().set( - &DataKey::MessageApproval(key), + storage::set_message_approval( + env, + message_approval_key, &Self::message_approval_hash(env, message.clone()), ); @@ -214,15 +214,16 @@ impl AxelarGatewayInterface for AxelarGateway { } fn epoch(env: &Env) -> u64 { - auth::epoch(env) + storage::epoch(env) } fn epoch_by_signers_hash(env: &Env, signers_hash: BytesN<32>) -> Result { - auth::epoch_by_signers_hash(env, signers_hash) + storage::try_epoch_by_signers_hash(env, signers_hash) + .ok_or(ContractError::InvalidSignersHash) } fn signers_hash_by_epoch(env: &Env, epoch: u64) -> Result, ContractError> { - auth::signers_hash_by_epoch(env, epoch) + storage::try_signers_hash_by_epoch(env, epoch).ok_or(ContractError::InvalidEpoch) } fn validate_proof( @@ -246,15 +247,7 @@ impl AxelarGateway { message_id, }; - Self::message_approval_by_key(env, key) - } - - /// Get the message approval value by key, defaulting to `MessageNotApproved` - fn message_approval_by_key(env: &Env, key: MessageApprovalKey) -> MessageApprovalValue { - env.storage() - .persistent() - .get(&DataKey::MessageApproval(key)) - .unwrap_or(MessageApprovalValue::NotApproved) + storage::try_message_approval(env, key).unwrap_or(MessageApprovalValue::NotApproved) } fn message_approval_hash(env: &Env, message: Message) -> MessageApprovalValue { diff --git a/contracts/stellar-axelar-gateway/src/lib.rs b/contracts/stellar-axelar-gateway/src/lib.rs index 9f7cb499..b1e8321e 100644 --- a/contracts/stellar-axelar-gateway/src/lib.rs +++ b/contracts/stellar-axelar-gateway/src/lib.rs @@ -28,7 +28,7 @@ cfg_if::cfg_if! { } else { mod auth; pub mod event; - mod storage_types; + mod storage; mod contract; pub use contract::{AxelarGateway, AxelarGatewayClient}; diff --git a/contracts/stellar-axelar-gateway/src/storage.rs b/contracts/stellar-axelar-gateway/src/storage.rs new file mode 100644 index 00000000..8b4663e4 --- /dev/null +++ b/contracts/stellar-axelar-gateway/src/storage.rs @@ -0,0 +1,55 @@ +use soroban_sdk::{contracttype, BytesN, String}; +use stellar_axelar_std::contractstorage; + +#[contracttype] +#[derive(Clone, Debug)] +pub struct MessageApprovalKey { + pub source_chain: String, + pub message_id: String, +} + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum MessageApprovalValue { + NotApproved, + Approved(BytesN<32>), + Executed, +} + +#[contractstorage] +#[derive(Clone, Debug)] +pub enum DataKey { + #[persistent] + #[value(MessageApprovalValue)] + MessageApproval { + message_approval_key: MessageApprovalKey, + }, + + #[instance] + #[value(u64)] + PreviousSignerRetention, + + #[instance] + #[value(BytesN<32>)] + DomainSeparator, + + #[instance] + #[value(u64)] + MinimumRotationDelay, + + #[instance] + #[value(u64)] + Epoch, + + #[instance] + #[value(u64)] + LastRotationTimestamp, + + #[persistent] + #[value(BytesN<32>)] + SignersHashByEpoch { epoch: u64 }, + + #[persistent] + #[value(u64)] + EpochBySignersHash { signers_hash: BytesN<32> }, +} diff --git a/contracts/stellar-axelar-gateway/src/storage_types.rs b/contracts/stellar-axelar-gateway/src/storage_types.rs deleted file mode 100644 index 20a4caf5..00000000 --- a/contracts/stellar-axelar-gateway/src/storage_types.rs +++ /dev/null @@ -1,31 +0,0 @@ -use soroban_sdk::{contracttype, BytesN, String}; - -#[contracttype] -#[derive(Clone, Debug)] -pub struct MessageApprovalKey { - pub source_chain: String, - pub message_id: String, -} - -#[contracttype] -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum MessageApprovalValue { - NotApproved, - Approved(BytesN<32>), - Executed, -} - -#[contracttype] -#[derive(Clone, Debug)] -pub enum DataKey { - /// Gateway - MessageApproval(MessageApprovalKey), - /// Auth Module - PreviousSignerRetention, - DomainSeparator, - MinimumRotationDelay, - Epoch, - LastRotationTimestamp, - SignersHashByEpoch(u64), - EpochBySignersHash(BytesN<32>), -} diff --git a/contracts/stellar-axelar-gateway/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden b/contracts/stellar-axelar-gateway/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden new file mode 100644 index 00000000..146ecda6 --- /dev/null +++ b/contracts/stellar-axelar-gateway/src/testdata/ensure_data_key_storage_schema_is_unchanged.golden @@ -0,0 +1,35 @@ +#[derive(Clone, Debug)] +pub enum DataKey { + + #[persistent] + #[value(MessageApprovalValue)] + MessageApproval { message_approval_key: MessageApprovalKey }, + + #[instance] + #[value(u64)] + PreviousSignerRetention, + + #[instance] + #[value(BytesN<32>)] + DomainSeparator, + + #[instance] + #[value(u64)] + MinimumRotationDelay, + + #[instance] + #[value(u64)] + Epoch, + + #[instance] + #[value(u64)] + LastRotationTimestamp, + + #[persistent] + #[value(BytesN<32>)] + SignersHashByEpoch { epoch: u64 }, + + #[persistent] + #[value(u64)] + EpochBySignersHash { signers_hash: BytesN<32> }, +} From 71a8cae51792ec812f2cca073a44836b825a97d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Noah=20B=2E=20=F0=9F=A5=82?= <15343884+nbayindirli@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:37:15 -0800 Subject: [PATCH 6/6] refactor(upgrader): use contractstorage (#247) --- Cargo.lock | 1 + contracts/stellar-upgrader/Cargo.toml | 1 + contracts/stellar-upgrader/src/lib.rs | 4 ++++ .../src/tests/atomic_upgrades.rs | 5 ++-- .../src/tests/utils/dummy_contract.rs | 18 ++++++++++----- .../utils/dummy_contract_after_upgrade.rs | 23 +++++++++++-------- ...ata_key_storage_schema_is_unchanged.golden | 6 +++++ 7 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 contracts/stellar-upgrader/src/tests/utils/testdata/ensure_data_key_storage_schema_is_unchanged.golden diff --git a/Cargo.lock b/Cargo.lock index 239c255d..ae0dd0de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2105,6 +2105,7 @@ name = "stellar-upgrader" version = "1.0.0" dependencies = [ "cfg-if", + "goldie", "soroban-sdk", "stellar-axelar-std", ] diff --git a/contracts/stellar-upgrader/Cargo.toml b/contracts/stellar-upgrader/Cargo.toml index 8f58907f..1ca65b43 100644 --- a/contracts/stellar-upgrader/Cargo.toml +++ b/contracts/stellar-upgrader/Cargo.toml @@ -15,6 +15,7 @@ soroban-sdk = { workspace = true } stellar-axelar-std = { workspace = true } [dev-dependencies] +goldie = { workspace = true } soroban-sdk = { workspace = true, features = ["testutils"] } [features] diff --git a/contracts/stellar-upgrader/src/lib.rs b/contracts/stellar-upgrader/src/lib.rs index c91b1ca1..a8288af0 100644 --- a/contracts/stellar-upgrader/src/lib.rs +++ b/contracts/stellar-upgrader/src/lib.rs @@ -1,4 +1,8 @@ #![no_std] + +#[cfg(any(test, feature = "testutils"))] +extern crate std; + #[cfg(test)] extern crate alloc; diff --git a/contracts/stellar-upgrader/src/tests/atomic_upgrades.rs b/contracts/stellar-upgrader/src/tests/atomic_upgrades.rs index 7c78bac8..890d4290 100644 --- a/contracts/stellar-upgrader/src/tests/atomic_upgrades.rs +++ b/contracts/stellar-upgrader/src/tests/atomic_upgrades.rs @@ -2,8 +2,9 @@ use soroban_sdk::testutils::Address as _; use soroban_sdk::{Address, BytesN, Env, String}; use stellar_axelar_std::{assert_contract_err, mock_auth}; -use super::utils::{DataKey, DummyContract, DummyContractClient}; +use super::utils::{DummyContract, DummyContractClient}; use crate::error::ContractError; +use crate::tests::utils; use crate::{Upgrader, UpgraderClient}; const WASM_AFTER_UPGRADE: &[u8] = include_bytes!("testdata/dummy.wasm"); @@ -42,7 +43,7 @@ fn upgrade_and_migrate_are_atomic() { // ensure migration was successful env.as_contract(&contract_address, || { - let data: String = env.storage().instance().get(&DataKey::Data).unwrap(); + let data = utils::storage::data(&env); assert_eq!(data, expected_data); }); } diff --git a/contracts/stellar-upgrader/src/tests/utils/dummy_contract.rs b/contracts/stellar-upgrader/src/tests/utils/dummy_contract.rs index f0e2b7b2..7b0606a6 100644 --- a/contracts/stellar-upgrader/src/tests/utils/dummy_contract.rs +++ b/contracts/stellar-upgrader/src/tests/utils/dummy_contract.rs @@ -1,8 +1,8 @@ //! Dummy contract to test the [crate::Upgrader] use soroban_sdk::{contract, contracterror, contractimpl, contracttype, Address, BytesN, Env}; -use stellar_axelar_std::interfaces; use stellar_axelar_std::interfaces::{OwnableInterface, UpgradableInterface}; +use stellar_axelar_std::{contractstorage, interfaces}; #[contract] pub struct DummyContract; @@ -38,12 +38,18 @@ impl DummyContract { } } -#[contracttype] -pub enum DataKey { - Data, -} - #[contracterror] pub enum ContractError { SomeFailure = 1, } + +pub mod storage { + use super::*; + + #[contractstorage] + pub enum DataKey { + #[instance] + #[value(soroban_sdk::String)] + Data, + } +} diff --git a/contracts/stellar-upgrader/src/tests/utils/dummy_contract_after_upgrade.rs b/contracts/stellar-upgrader/src/tests/utils/dummy_contract_after_upgrade.rs index 2c1f9a4b..f99ad4c9 100644 --- a/contracts/stellar-upgrader/src/tests/utils/dummy_contract_after_upgrade.rs +++ b/contracts/stellar-upgrader/src/tests/utils/dummy_contract_after_upgrade.rs @@ -1,8 +1,8 @@ //! Base for the dummy.wasm file. This is the dummy contract after upgrade. use soroban_sdk::{contract, contracterror, contractimpl, contracttype, Address, BytesN, Env}; -use stellar_axelar_std::interfaces; use stellar_axelar_std::interfaces::{OwnableInterface, UpgradableInterface}; +use stellar_axelar_std::{contractstorage, interfaces}; #[contract] pub struct DummyContract; @@ -39,19 +39,24 @@ impl DummyContract { pub fn migrate(env: Env, migration_data: soroban_sdk::String) -> Result<(), ContractError> { Self::owner(&env).require_auth(); - env.storage() - .instance() - .set(&DataKey::Data, &migration_data); + storage::set_data(&env, &migration_data); + Ok(()) } } -#[contracttype] -pub enum DataKey { - Data, -} - #[contracterror] pub enum ContractError { SomeFailure = 1, } + +mod storage { + use super::*; + + #[contractstorage] + pub enum DataKey { + #[instance] + #[value(soroban_sdk::String)] + Data, + } +} diff --git a/contracts/stellar-upgrader/src/tests/utils/testdata/ensure_data_key_storage_schema_is_unchanged.golden b/contracts/stellar-upgrader/src/tests/utils/testdata/ensure_data_key_storage_schema_is_unchanged.golden new file mode 100644 index 00000000..75a133d8 --- /dev/null +++ b/contracts/stellar-upgrader/src/tests/utils/testdata/ensure_data_key_storage_schema_is_unchanged.golden @@ -0,0 +1,6 @@ +pub enum DataKey { + + #[instance] + #[value(soroban_sdk::String)] + Data, +}