feat: pass powertable to blobs actor by ukstv · Pull Request #263 · recallnet/ipc

ukstv · 2024-10-16T17:57:22Z

UPD after the comments and discussion with Will from IPC:

send a tx to Blobs actor in the end block of ExecInterpreter for FvmMessageInterpreter,
send updates already prepared by the existing code (higher level than StakingChange or StakingOperation.

ukstv · 2024-10-16T17:57:48Z

fendermint/vm/interpreter/src/chain.rs

                    let proposer = state.validator_id().map(|id| id.to_string());
                    let proposer_ref = proposer.as_deref();

+                    let (_configuration_number, powers) = self.gateway_caller.current_power_table(&mut state)?;


Is it an okay place to call the actor?

yep, this is the right spot...

I wonder though, instead of re-fetching the full table, can you just send the validator_changes to the blob actor? they look like,

pub struct StakingChangeRequest { pub configuration_number: ConfigurationNumber, pub change: StakingChange, }

I don't think we care about configuration_number in this context.

where StakingChange is,

pub struct StakingChange { pub op: StakingOperation, pub payload: Vec<u8>, pub validator: Address, }

payload can be ignored, which is the pubkey.

and StakingOperation is,

pub enum StakingOperation { Deposit = 0, Withdraw = 1, SetMetadata = 2, SetFederatedPower = 3, }

I don't think we care about SetMetadata or SetFederatedPower. so you could filter those out of the changes.

The logic in the actor would reconcile the changes into an "accumulated" power table.

ukstv · 2024-10-16T18:00:09Z

@sanderpick Please, have a look at this attempt to pass power table to the blobs actor. It is dirty, but should (probably) work. The biggest uncertainty is where to invoke the call. You definitely know better if "deliver" of ExecInterpreter the right place.

sanderpick

Looking good, left some comments

sanderpick · 2024-10-17T22:50:36Z

fendermint/vm/interpreter/src/chain.rs

                    let proposer = state.validator_id().map(|id| id.to_string());
                    let proposer_ref = proposer.as_deref();

+                    let (_configuration_number, powers) = self.gateway_caller.current_power_table(&mut state)?;


yep, this is the right spot...

I wonder though, instead of re-fetching the full table, can you just send the validator_changes to the blob actor? they look like,

pub struct StakingChangeRequest { pub configuration_number: ConfigurationNumber, pub change: StakingChange, }

I don't think we care about configuration_number in this context.

where StakingChange is,

pub struct StakingChange { pub op: StakingOperation, pub payload: Vec<u8>, pub validator: Address, }

payload can be ignored, which is the pubkey.

and StakingOperation is,

pub enum StakingOperation { Deposit = 0, Withdraw = 1, SetMetadata = 2, SetFederatedPower = 3, }

I don't think we care about SetMetadata or SetFederatedPower. so you could filter those out of the changes.

The logic in the actor would reconcile the changes into an "accumulated" power table.

sanderpick · 2024-10-17T22:52:04Z

fendermint/vm/interpreter/src/chain.rs

+        let (mut state, out) = self.inner.end(state).await?;
+
+        // TODO SU Wrong
+        let a = self.gateway_caller.current_power_table(&mut state);


yep, not needed here

sanderpick · 2024-10-17T22:53:14Z

fendermint/vm/interpreter/src/chain.rs

            })
            .await;
+
+            // TODO Continue update power table in blobs actor


not needed if doing in the txn deliver handler. this end block is where you'd update any off-chain components, like env.parent_finality_votes.

sanderpick · 2024-10-22T04:54:47Z

fendermint/vm/interpreter/src/fvm/exec.rs

+            .iter()
+            .filter_map(|validator| {
+                let public_key = validator.public_key.0.serialize();
+                let address = Address::new_secp256k1(&public_key);


i think this address is going to be of the f1-style. the validators will be f410 style with a delegated ethereum addresses. ie, i don't think this format is what we want on-chain because we won't be able to allow claims using this address format. this is one reason why i was suggesting using the power table updates that come from top down messages, because they already include the correct validator address. that being said, I haven't had a chance to really look at this... I got sucked into CI fixes today. i'll take another look tomorrow... I think you could continue with next steps on top of this though.

oop, I saw your slack thread with the IPC team... sounds like you're more informed than me on how to do this now. btw, we just merged the gas market actor to develop, so you could mimic / piggy back on that. though i don't see where the power table is stored in the new actor.

Ok, I figured. There are few layers of indirection between a pub key and the address.

ukstv · 2024-10-28T10:14:31Z

I think I figured the addresses.

Assumption: a validator uses the same (public) key for CometBFT consensus and for interaction with the chain.

If the assumption is true, then we convert the CometBFT public key to ETH address, and then convert it into delegated address through EAM actor (i.e. actor id of the address manager who converts between the "external" address and an address internal to FVM, see FIP-48), where "payload" is eth address bytes.

avichalp · 2024-10-31T00:27:50Z

fendermint/vm/interpreter/src/fvm/exec.rs

            checkpoint::maybe_create_checkpoint(&self.gateway, &mut state)
                .context("failed to create checkpoint")?
        {
+            let power_table = prepare_blobs_power_table(&updates);


Is there a particular reason to execute this transaction in ExecInterpreter instead of ChainMessageInterpreter? I am asking because we have been generally putting all such changes where we have to execute implicit transaction in ChainMessageInterpreter.

It is just a place where updates are readily available. Other than that, no reason.

That is, I am all right to move it there, according to the tradition, unless I find any insurmountable resistance on the code level. And, I'll tell you if that happens :)

Thank you for the clue!

sounds good. i think, it would be easier to track changes that we have made in fendermint if they are all in the same place.

@avichalp I believe now it belongs to the right place.

ukstv · 2024-10-31T16:37:45Z

Oh, and converting to the draft, as it is not urgently needed.

ukstv commented Oct 16, 2024

View reviewed changes

sanderpick reviewed Oct 17, 2024

View reviewed changes

ukstv force-pushed the su/pass-powertable-to-blobs-actor branch from 3b8a119 to f5a3616 Compare October 21, 2024 06:24

ukstv changed the title ~~Su/pass powertable to blobs actor~~ feat: pass powertable to blobs actor Oct 21, 2024

ukstv requested a review from sanderpick October 21, 2024 17:43

ukstv marked this pull request as ready for review October 21, 2024 17:46

sanderpick reviewed Oct 22, 2024

View reviewed changes

ukstv force-pushed the su/pass-powertable-to-blobs-actor branch from 8ed1261 to 50f8498 Compare October 25, 2024 16:15

ukstv requested review from avichalp and carsonfarmer October 29, 2024 12:13

ukstv force-pushed the su/pass-powertable-to-blobs-actor branch from bfd316b to f05c620 Compare October 29, 2024 12:14

avichalp reviewed Oct 31, 2024

View reviewed changes

ukstv marked this pull request as draft October 31, 2024 16:37

ukstv added 14 commits November 4, 2024 12:48

aha

239bf6d

add powertable

8c3345a

maybe save the powertable

bedf070

drop dev comments

ab40332

drop dev comments

f5ff19d

lint

a45e47a

wip end block power table update

b3702ce

wip end block power table update

aa0c6e0

moved powertable around

e63e712

linter

e6404a0

update power table entries

ae5fa38

wip

816adb8

wip

2f276a2

chore: eth address

b246515

ukstv added 4 commits November 4, 2024 12:48

chore: eth address, mapped

cdfa26b

respect linter

54b3e23

collect validator

76dc1e0

lint

4031262

ukstv force-pushed the su/pass-powertable-to-blobs-actor branch from 95f5b70 to 4031262 Compare November 4, 2024 09:49

sanderpick added the archived Archived for now label Mar 12, 2025

Conversation

ukstv commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukstv commented Oct 16, 2024

Uh oh!

sanderpick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukstv commented Oct 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukstv Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukstv Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukstv commented Oct 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ukstv commented Oct 16, 2024 •

edited

Loading

ukstv Oct 31, 2024 •

edited

Loading

ukstv Oct 31, 2024 •

edited

Loading