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

BlockHeader algod v2 misses documentation #19

Closed
manuelmauro opened this issue May 5, 2021 · 3 comments
Closed

BlockHeader algod v2 misses documentation #19

manuelmauro opened this issue May 5, 2021 · 3 comments
Labels
good first issue Good for newcomers

Comments

@manuelmauro
Copy link
Owner

Add documentation to the fields composing BlockHeader struct plus check which are optional and correct the type signature.

@manuelmauro manuelmauro added the good first issue Good for newcomers label May 5, 2021
@epequeno
Copy link
Contributor

epequeno commented Jun 17, 2021

I'd like to pick this one up, I think I'll need a bit more info to start on this though.

This is the BlockHeader in algod we should be looking at right?
https://github.com/algorand/go-algorand/blob/91b8ca1ed7677f41815c06d7808a17077941c73b/data/bookkeeping/block.go#L38

Which I believe correlates to this struct in the algod messages module:

/// BlockHeader
#[derive(Debug, Serialize, Deserialize)]
pub struct BlockHeader {
pub earn: Option<u64>,
pub fees: String,
pub frac: u64,
pub gen: String,
pub gh: String,
pub prev: String,
pub proto: String,
pub rate: u64,
pub rnd: u64,
pub rwcalr: u64,
pub rwd: String,
pub seed: String,
pub ts: u64,
pub txn: String,
}

I've tried to line these fields up side by side but it reveals that I'm not sure exactly how to map them to each other:

earn: Option<u64>,
fees: String,
frac: u64,
gen: String,     GenesisID string
gh: String,      GenesisHash crypto.Digest
prev: String,
proto: String,
rate: u64,
rnd: u64,        Round basics.Round
rwcalr: u64,
rwd: String,     RewardsState
seed: String,    Seed committee.Seed
ts: u64,         TimeStamp int64
txn: String,     TxnRoot crypto.Digest
                 Branch BlockHash
                 UpgradeState
                 UpgradeVote
                 TxnCounter uint64
                 CompactCert map[protocol.CompactCertType]CompactCertState

I'd like to check in and confirm if I'm on the right track and to ask if you could provide any further guidance on how to proceed.

@manuelmauro
Copy link
Owner Author

Hi @epequeno ! Yes, it's quite tricky to find documentation about this one. The current struct is defined starting from a test call towards the API, which makes it a very poor solution. We don't know which fields are optional and which are not for instance.

Your reference seems to be the best source I am able to find. Another source comes from an automatic implementation of the client based on the Open API Schema that suggests the following implementation for the API response (completely avoiding the problem):

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct InlineResponse2001 {
    /// Block header data.
    #[serde(rename = "block")]
    pub block: serde_json::Value,
    /// Optional certificate object. This is only included when the format is set to message pack.
    #[serde(rename = "cert", skip_serializing_if = "Option::is_none")]
    pub cert: Option<serde_json::Value>,
}

If we are not able to find solid docs this solution might be the less error-prone.

@ivnsch
Copy link
Contributor

ivnsch commented Apr 23, 2022

Closing this, as it seems not urgent and it would be solved as part of #164 and #165

@ivnsch ivnsch closed this as completed Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants