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

fetch weights from remote node #12

Open
niklasad1 opened this issue Jun 1, 2022 · 5 comments
Open

fetch weights from remote node #12

niklasad1 opened this issue Jun 1, 2022 · 5 comments

Comments

@niklasad1
Copy link
Member

niklasad1 commented Jun 1, 2022

Currently, the weights are hardcoded for each chain because the MinerConfig trait from substrate is required to be implemented.

Instead ideally it would be useful if we could just get the weights via runtime RPC API.
Perhaps something like:

   fn weight_from_extrinsic(uxt: UnsignedExtrinsic) -> Weight

Similar to https://github.com/paritytech/polkadot/blob/master/runtime/polkadot/src/weights/pallet_election_provider_multi_phase.rs#L142-#L153

Then just plug add the Weight as a parameter to the call to mine_solution_with_snapshot.

Which in turn requires changes in EPM pallet to remove solution_weight from the Miner trait.

That pretty much would make it work.

paritytech/substrate#11665 might help with this.

/cc @kianenigma

@niklasad1 niklasad1 changed the title add a way to sync weights from the runtimes dynamically sync weights from the runtimes dynamically Jun 1, 2022
@niklasad1 niklasad1 changed the title sync weights from the runtimes dynamically fetch weights from remote node Jul 7, 2022
@kianenigma
Copy link
Contributor

Optionally, fn solution_weight should stay, instead in the impl MinerConfig for _ in the staking miner, instead we forward the call to the RPC. Might clash a bit with mixing async functions with a normal function, wdyt?

@niklasad1
Copy link
Member Author

niklasad1 commented Aug 15, 2022

it's a bit difficult to make fn solution_weight a future or something like that because the EPM pallet uses that API and it needs to be implemented for the runtime mocks.

It's probably easier to remove some of the things from MinerConfig and try what I wrote above, it's a bit more work than I expected :P

@niklasad1
Copy link
Member Author

niklasad1 commented Aug 16, 2022

After getting some help by Kian and we discussed the to create a "fake RawSolution" with correct data from the snapshot.
Example snippet code in staking miner how to do it:

    let (voters, targets, desired_targets) = snapshot(&api, hash).await?;

    // voters, targets and desired_targets should be used in here.
    let raw_solution = Default::default();
    let uxt = api
        .tx()
        .election_provider_multi_phase()
        .submit(raw_solution)?;
    let xt = uxt
        .create_signed(&*signer, ExtrinsicParams::default())
        .await?;

    let encoded_xt = Bytes(xt.encoded().to_vec());

    let weight: Weight = api
        .client
        .rpc()
        .client
        .request(
            "state_call",
            rpc_params!["TransactionPaymentApi_query_fee_details", encoded_xt],
        )
        .await
        .unwrap();

  // this is currently not possible and doesn't work because `EPM::maximum_voter_for_weight`     
  // calls`MinerConfig::solution_weight several times this just has a static weight.....
   MinerConfig::mine_solution_with_snapshot(
        voters,
        targets,
        desired_targets,
        weight
    )

In order to have this API in miner then the EPM pallet would need to be changed to take Weight as input and it seems not like a super good idea because EPM::maximum_voter_for_weight actually calls MinerConfig::solution_weight more than once with the different values.

Thus, it could be possible move maximum_voter_for_weight to the staking-miner side of things and then just pass in maximum_allowed_voters in mine_solution_with_snapshot instead of the weight.

//cc @kianenigma thoughts on doing that?

@jsdw jsdw removed this from the Staking miner v2 production ready milestone Aug 16, 2022
@niklasad1
Copy link
Member Author

niklasad1 commented Aug 17, 2022

Thus the proposed could would look something like:

    pub async fn maximum_voter_for_weight(
                &self,
		desired_winners: u32,
		size: SolutionOrSnapshotSize,
		max_weight: Weight,
	) -> u32 {

     // this could be closure but async closures doesn't work
     async fn with_weight(rpc, voters: u32, targets: u32, active_voters: u32, desired_targets: u32) -> Weight {
          let solution = from(voters, targets, active_voters, desired_targets);
          let uxt = api.tx().election_provider_multi_phase().submit(raw_solution).encode();
          rpc.request("state_call". rpc_params!["TransactionPaymentApi_query_fee_details", Bytes::new(uxt)
      }

      ... the existing code with binary search etc....
      ... that calls `with_weight`
    }



    let (voters, targets, desired_targets) = snapshot(&api, hash).await?;

    let size = SolutionOrSnapshotSize { voters: voters.len() as u32, targets: targets.len() as u32 };
    let maximum_allowed_voters = maximum_voter_for_weight(voters, size, MinerConfig::MaxWeight);

   MinerConfig::mine_solution_with_snapshot(
        voters,
        targets,
        desired_targets,
        maximum_allowed_voters,
    )

it looks a bit hacky but that's simplest I could come up with.

@kianenigma
Copy link
Contributor

This is done, but likely it is super duper slow, so will leave open for now.

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

No branches or pull requests

3 participants