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

Block extension data available via SHiP but not in ABI #720

Closed
n8d opened this issue Sep 8, 2024 · 8 comments · Fixed by #783 or #812
Closed

Block extension data available via SHiP but not in ABI #720

n8d opened this issue Sep 8, 2024 · 8 comments · Fixed by #783 or #812
Assignees
Labels
enhancement New feature or request 👍 lgtm
Milestone

Comments

@n8d
Copy link

n8d commented Sep 8, 2024

It seems we have a new block_extension (type 3) in the SHiP stream, but I'm unable to deserialize it because it's not a type that is included in the ABI received on connecting to SHiP.

I'm actually not sure if type 2 is in the ABI as I've not tried to deserialize that one, but in general it seems if data is there we should be able to deserialize it.

@enf-ci-bot enf-ci-bot moved this to Todo in Team Backlog Sep 8, 2024
@arhag arhag added the enhancement New feature or request label Sep 9, 2024
@spoonincode
Copy link
Member

As far as I can tell this isn't anything new -- other extensions aren't explicitly communicated in the ABI either. For example it seems for block header extension 0 one would need to manually know that when seeing extension.type: 0 then extension.bytes would need to be deserialized as checksum256[] -- this isn't communicated via the ABI.

I wonder if maybe we could change the various extensions in the ABI to appropriate variants. This would make accessing those extensions more seamless, but of course this would be a very breaking change that would require some sort of opt-in mechanism from the client. The main problem is that the block extension IDs are sparse which makes using a variant not a perfect fit.

Alternatively we can simply include the new extension types in the ABI and leave it up to the client to manually know which types goes with which ID.

@n8d
Copy link
Author

n8d commented Sep 10, 2024

Yeah it's not been explicit for others, but it's been possible, at least for the extensions I've needed. For example header extension 1 can be deserialized with producer_authority_schedule. Typically I will brute force it by trying to deserialize it with every type in the ABI until I find one that looks right, but I'm not having any luck with this one.

That said, I'm not actually sure if block extension 3 has what I need (because I haven't been able to view it). I'm trying to find where the new producer schedules are coming across in savanna blocks (or headers).

@heifner
Copy link
Member

heifner commented Sep 10, 2024

3 is a quorum_certificate_extension

struct quorum_certificate_extension : fc::reflect_init {

@n8d
Copy link
Author

n8d commented Sep 10, 2024

Ok, so not seeing a producer schedule there. If they are not in the original new_producers, nor header extension 1, where are they now?

@greg7mdp
Copy link
Contributor

greg7mdp commented Sep 10, 2024

In the finality_extension, block header extension id 2. See finality_extension.hpp. Member new_proposer_policy_diff.

@heifner
Copy link
Member

heifner commented Sep 10, 2024

They are in the block header extension with id 2, finality_extension, when they are proposed.

struct finality_extension : fc::reflect_init {

Note it is a diff of the new proposer policy to the old proposer policy. The diff is created by:

@n8d
Copy link
Author

n8d commented Sep 10, 2024

Thanks, but now it seems we are back to the original title of the GitHub issue with a different extension. 😆

I tested every type in the ABI and I'm unable to deserialize block header extension 2.

@arhag
Copy link
Member

arhag commented Sep 23, 2024

We will add extra types in the SHiP ABI to allow deserializing the block header extension (this is added by #783). But we decided to not add support for deserializing the block extension because of the problems introduced by the serialization of the dynamic bitset.

@spoonincode spoonincode moved this from Todo to Awaiting Review in Team Backlog Sep 23, 2024
@arhag arhag linked a pull request Sep 23, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Team Backlog Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment