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

Commit Reveal Weights #389

Closed
distributedstatemachine opened this issue May 1, 2024 · 2 comments
Closed

Commit Reveal Weights #389

distributedstatemachine opened this issue May 1, 2024 · 2 comments
Assignees
Labels
breaking-change This PR introduces a noteworthy breaking change red team feature development, etc

Comments

@distributedstatemachine
Copy link
Contributor

Decscription

The goal is to implement a commit-reveal scheme for submitting weights in the Subtensor module. This scheme will require validators to submit a hashed version of their weights along with a signature during the commit phase. After a specified number of blocks (reveal tempo), validators will reveal the actual weights, which will be verified against the commit hash.

Requirements

  1. Implement a commit_weights function that allows validators to submit a hashed version of their weights along with a signature during the commit phase.
  2. Implement a reveal_weights function that allows validators to reveal the actual weights after the specified reveal tempo.
  3. Ensure that commits and reveals are unique to each validator and include a signature to prevent copying.
  4. Make the commit-reveal scheme optional and configurable per subnet.
  5. Enforce the reveal tempo and allow commits only once within a tempo period.
  6. Implement helper functions for the commit-reveal process and background tasks.
  7. Provide clear error messages for issues like missing commits, hash mismatches, or timing violations.

Rust Implementation

Storage

Add the following storage item to store commit hashes, signatures, and block numbers per validator and subnet:

#[pallet::storage]
pub type WeightCommits<T: Config> = StorageDoubleMap<_, Twox64Concat, u16, Twox64Concat, T::AccountId, (T::Hash, T::Signature, T::BlockNumber), ValueQuery>;

commit_weights Function

Implement the commit_weights function for the commit phase:

#[pallet::call]
impl<T: Config> Pallet<T> {
    pub fn commit_weights(
        origin: T::RuntimeOrigin,
        netuid: u16,
        commit_hash: T::Hash,
        signature: T::Signature,
    ) -> DispatchResult {
        let who = ensure_signed(origin)?;
        ensure!(Self::can_commit(netuid, &who), Error::<T>::CommitNotAllowed);
        WeightCommits::<T>::insert(netuid, &who, (commit_hash, signature, <frame_system::Pallet<T>>::block_number()));
        Ok(())
    }
}

reveal_weights Function

Implement the reveal_weights function for the reveal phase:

pub fn reveal_weights(
    origin: T::RuntimeOrigin,
    netuid: u16,
    uids: Vec<u16>,
    values: Vec<u16>,
    version_key: u64,
) -> DispatchResult {
    let who = ensure_signed(origin)?;
    WeightCommits::<T>::try_mutate_exists(netuid, &who, |maybe_commit| -> DispatchResult {
        let (commit_hash, signature, commit_block) = maybe_commit.take().ok_or(Error::<T>::NoCommitFound)?;
        ensure!(Self::is_reveal_block(netuid, commit_block), Error::<T>::InvalidRevealTempo);
        let provided_hash = T::Hashing::hash_of(&(uids.clone(), values.clone(), version_key));
        ensure!(provided_hash == commit_hash, Error::<T>::InvalidReveal);
        ensure!(Self::verify_signature(&who, &commit_hash, &signature), Error::<T>::InvalidSignature);
        Self::do_set_weights(
            T::Origin::from(origin),
            netuid,
            uids,
            values,
            version_key,
        )
    })
}

Helper Functions

Implement helper functions for the commit-reveal process:

impl<T: Config> Pallet<T> {
    fn can_commit(netuid: u16, who: &T::AccountId) -> bool {
        // Check if commit-reveal is enabled for the subnet
        // Check if the validator hasn't committed within the current tempo```rust
        // ...
    }

    fn is_reveal_block(netuid: u16, commit_block: T::BlockNumber) -> bool {
        // Check if the current block is within the reveal tempo
        // ...
    }

    fn verify_signature(who: &T::AccountId, commit_hash: &T::Hash, signature: &T::Signature) -> bool {
        // Verify the provided signature against the commit hash and validator's public key
        // ...
    }
}

Implement background tasks for cleaning up expired commits and managing the commit-reveal process.

Python Implementation

commit_weights Function

Add to bittensor/subtensor.py:

def commit_weights(
    self,
    wallet: "bittensor.wallet",
    netuid: int,
    commit_hash: str,
    signature: str,
    wait_for_inclusion: bool = False,
    wait_for_finalization: bool = False,
) -> Tuple[bool, Optional[str]]:
    @retry(delay=2, tries=3, backoff=2, max_delay=4)
    def make_substrate_call_with_retry():
        with self.substrate as substrate:
            call = substrate.compose_call(
                call_module="SubtensorModule",
                call_function="commit_weights",
                call_params={
                    "netuid": netuid,
                    "commit_hash": commit_hash,
                    "signature": signature,
                },
            )
            extrinsic = substrate.create_signed_extrinsic(call=call, keypair=wallet.coldkey)
            response = substrate.submit_extrinsic(
                extrinsic,
                wait_for_inclusion=wait_for_inclusion,
                wait_for_finalization=wait_for_finalization,
            )
            if not wait_for_finalization and not wait_for_inclusion:
                return True, None
            response.process_events()
            if response.is_success:
                return True, None
            else:
                return False, response.error_message

    return make_substrate_call_with_retry()

reveal_weights Function

Add to bittensor/subtensor.py:

def reveal_weights(
    self,
    wallet: "bittensor.wallet",
    netuid: int,
    uids: List[int],
    values: List[int],
    version_key: int,
    wait_for_inclusion: bool = False,
    wait_for_finalization: bool = False,
) -> Tuple[bool, Optional[str]]:
    @retry(delay=2, tries=3, backoff=2, max_delay=4)
    def make_substrate_call_with_retry():
        with self.substrate as substrate:
            call = substrate.compose_call(
                call_module="SubtensorModule",
                call_function="reveal_weights",
                call_params={
                    "netuid": netuid,
                    "uids": uids,
                    "values": values,
                    "version_key": version_key,
                },
            )
            extrinsic = substrate.create_signed_extrinsic(call=call, keypair=wallet.coldkey)
            response = substrate.submit_extrinsic(
                extrinsic,
                wait_for_inclusion=wait_for_inclusion,
                wait_for_finalization=wait_for_finalization,
            )
            if not wait_for_finalization and not wait_for_inclusion:
                return True, None
            response.process_events()
            if response.is_success:
                return True, None
            else:
                return False, response.error_message

    return make_substrate_call_with_retry()

Helper Functions

Add to bittensor/subtensor.py:

def can_commit(self, netuid: int, who: str) -> bool:
    # Check if commit-reveal is enabled for the subnet
    # Check if the validator hasn't committed within```python
    # Check if the validator hasn't committed within the current tempo
    # ...
    pass

def is_reveal_block(self, netuid: int, commit_block: int) -> bool:
    # Check if the current block is within the reveal tempo
    # ...
    pass

def verify_signature(self, who: str, commit_hash: str, signature: str) -> bool:
    # Verify the provided signature against the commit hash and validator's public key
    # ...
    pass

Implement background tasks for cleaning up expired commits and managing the commit-reveal process.

Error Handling

Provide clear error messages in bittensor/errors.py for various scenarios, such as:

  • Missing commits when revealing weights
  • Mismatched commit hash when revealing weights
  • Invalid signature when committing or revealing weights
  • Attempting to commit weights more than once within a tempo period
  • Attempting to reveal weights outside the reveal tempo

Testing

  • Write unit tests to cover the functionality of the commit_weights and reveal_weights functions in both Rust and Python.
  • Write integration tests to ensure the commit-reveal scheme works as expected with the Subtensor module.
  • Test various error scenarios and ensure appropriate error messages are provided.

Documentation

  • Update the Subtensor module's documentation to include information about the commit-reveal scheme.
  • Provide examples and guidelines for subnet owners to configure and use the commit-reveal scheme effectively.
  • Update the Subtensor Python module's documentation with information and examples about the commit-reveal scheme.

Const

  • Python side of the code to written subtensor.commit_weights extrinsics
  • Commits takes the reveal tempo, how long after in terms of blocks before we
  • We want the owner to be able set the value, min blocks that pass before you reveal.
  • We want to make them subtensor primitives
  • subtensor helper commit reveal + set background process
  • We want to be careful with tempo and only allow it to be called once within a tempo
  • focus on the primitives

Vune

  • We should use signatures of weights
  • make it optional
@gztensor
Copy link
Contributor

gztensor commented May 1, 2024

  1. Instead of using commit signature, can we hash the weights, netuid, and public key of hotkey all together? The extrinsic is already signed, and a separate signature is not needed.
  2. Should we include nonce in the plaintext before hashing too to avoid replay attacks (just in case extrinsic lifetime is longer than commit tempos)?

The process will look like this:

Commit:
(netuid, hotkey_pub, nonce, weights) -> hash -> commit_weights

Reveal:
(netuid, nonce, weights) -> reveal_weights -> (netuid, hotkey_pub, nonce, weights) -> hash -> verify

@distributedstatemachine
Copy link
Contributor Author

@sam0x17 breaking change

@distributedstatemachine distributedstatemachine added the breaking-change This PR introduces a noteworthy breaking change label May 4, 2024
@JohnReedV JohnReedV mentioned this issue May 5, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This PR introduces a noteworthy breaking change red team feature development, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants