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

Enhance Commit/Reveal Mechanism to Allow Multiple Pending Commits #598

Open
distributedstatemachine opened this issue Jul 1, 2024 · 0 comments · May be fixed by #599
Open

Enhance Commit/Reveal Mechanism to Allow Multiple Pending Commits #598

distributedstatemachine opened this issue Jul 1, 2024 · 0 comments · May be fixed by #599

Comments

@distributedstatemachine
Copy link
Contributor

Description

We need to modify our commit/reveal mechanism to allow validators to submit multiple weight commits without overwriting previous ones. The nonce for each commit will be generated by the Python client rather than on-chain.

Acceptance Criteria

  1. Validators should be able to submit multiple weight commits without overwriting previous pending commits.
  2. Each commit should be identified by a client-generated nonce.
  3. Validators should be able to reveal any of their pending commits within the reveal period.
  4. The system should maintain the existing time constraints for the reveal period.
  5. If a reveal is successful, only the revealed commit should be removed from the pending commits.

Tasks

  1. Modify the storage structure to allow multiple pending commits per validator.
#[pallet::storage]
pub type WeightCommits<T: Config> = StorageDoubleMap<
    _,
    Twox64Concat,
    u16, // netuid
    Twox64Concat,
    T::AccountId,
    BTreeMap<u64, (H256, u64)>, // nonce -> (hash, block_number)
    ValueQuery,
>;
  1. Update the do_commit_weights function to add new commits with client-provided nonces.
pub fn do_commit_weights(
    origin: T::RuntimeOrigin,
    netuid: u16,
    commit_hash: H256,
    nonce: u64,
) -> DispatchResult {
    let who = ensure_signed(origin)?;
    // ... existing checks ...

    WeightCommits::<T>::mutate(netuid, &who, |commits| {
        commits.insert(nonce, (commit_hash, frame_system::Pallet::<T>::block_number()));
    });

    Ok(())
}
  1. Modify the do_reveal_weights function to work with the new multi-commit system, allowing reveal of any valid pending commit.
pub fn do_reveal_weights(
    origin: T::RuntimeOrigin,
    netuid: u16,
    uids: Vec<u16>,
    values: Vec<u16>,
    salt: Vec<u16>,
    version_key: u64,
    nonce: u64,
) -> DispatchResult {
    let who = ensure_signed(origin.clone())?;
    
    WeightCommits::<T>::try_mutate_exists(netuid, &who, |maybe_commits| -> DispatchResult {
        let commits = maybe_commits.as_mut().ok_or(Error::<T>::NoWeightsCommitFound)?;
        
        let (commit_hash, commit_block) = commits.get(&nonce)
            .ok_or(Error::<T>::InvalidCommitNonce)?;

        ensure!(
            Self::is_reveal_block_range(netuid, *commit_block),
            Error::<T>::InvalidRevealCommitTempo
        );

        let provided_hash: H256 = BlakeTwo256::hash_of(&(
            who.clone(),
            netuid,
            uids.clone(),
            values.clone(),
            salt.clone(),
            version_key,
        ));
        ensure!(
            provided_hash == *commit_hash,
            Error::<T>::InvalidRevealCommitHashNotMatch
        );

        // All checks have passed, now we can set the weights
        Self::do_set_weights(origin, netuid, uids, values, version_key)?;

        // After successfully setting weights, remove the revealed commit
        commits.remove(&nonce);
        if commits.is_empty() {
            *maybe_commits = None;
        }

        Ok(())
    })
}

Additional Considerations

  • How will the CLI generate and manage nonces to ensure uniqueness?
  • Should there be a limit on the number of pending commits per validator to prevent potential spam or DoS attacks?
@distributedstatemachine distributedstatemachine linked a pull request Jul 1, 2024 that will close this issue
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant