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

fix: align with new API res #66

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

charmful0x
Copy link

@charmful0x charmful0x commented Nov 18, 2024

Update Blobscan API integration to match new schema

The Blobscan team has updated their API, removing DataStorageReference from the getBlockById method response. This PR aligns with new API schema at https://api.blobscan.com/#/blocks/block-getByBlockId :

  • Makes DataStorageReference fields optional (Option)
  • Updates the get_block_by_id test with a mainnet block hash

/// data location
pub data_reference: String,
pub data_reference: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should have #[serde(default, skip_serializing_if = "Option::is_none")] to actually make them optional

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed, thanks!

@klkvr
Copy link
Member

klkvr commented Nov 18, 2024

should we just remove the struct and data_storage_reference field here?

pub data_storage_references: Vec<DataStorageReference>,

@charmful0x
Copy link
Author

should we just remove the struct and data_storage_reference field here?

pub data_storage_references: Vec<DataStorageReference>,

hmm i thought of keeping because it could be added back as same as it got removed (?) so it gets caught in that case-- blobscan API seems to not be versioned so far, so for that reason (as it existed, it may return, or not)

But yeah happy to comment it out as well

@klkvr
Copy link
Member

klkvr commented Nov 18, 2024

let's keep it then and add serde(default, skip_serializing_if = "Vec::is_empty") to the field instead of entire struct as I'd expect the struct to be valid if it's present

@charmful0x
Copy link
Author

let's keep it then and add serde(default, skip_serializing_if = "Vec::is_empty") to the field instead of entire struct as I'd expect the struct to be valid if it's present

re-addressed!

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 this pull request may close these issues.

3 participants