blockfrost / blockfrost-rust

Rust SDK for Blockfrost.io
Apache License 2.0
15 stars 16 forks source link

Condensed endpoint declaration macro #3

Closed Quantumplation closed 2 years ago

Quantumplation commented 2 years ago

There's a lot of repetition in the endpoint declarations. Would you accept a PR that introduced a macro for making these easier to define?

image

It's a little DSL-y, but it sure cuts down on a ton of boilerplate, IMO

Still generates all the same code and documentation:

image

(I still have to update it to handle parameters like {pool_id}, but you get the idea)

marcospb19 commented 2 years ago

It's a good idea, I have two questions with regards to it:

  1. Some endpoints require different types in the path parameters:

Requires Integer: https://docs.blockfrost.io/#tag/Cardano-Blocks/paths/~1blocks~1slot~1{slot_number}/get

Requires String: https://docs.blockfrost.io/#tag/Cardano-Accounts/paths/~1accounts~1{stake_address}/get

Require Integer and String: https://docs.blockfrost.io/#tag/Cardano-Blocks/paths/~1blocks~1epoch~1{epoch_number}~1slot~1{slot_number}/get

Would this add too much complexity to the macro?

I wonder if this forces it to be complex and weirder to maintain than the current implementation.


  1. If default query parameters were to be implemented, and order is set to Descending at the creation of the BlockFrostApi, the suffix "?order=desc" should be added in every request? Even those who do not expect the count parameter?

If this is not desirable, we would need some way of telling each function what parameters should be inserted in the final URL. The easiest way would be using docs.rs/bitflags, and the macro would need to handle it too.

Quantumplation commented 2 years ago

Here it is with added parameters:

image

You just specify the list of parameters and types, and it expands out into parameters to the method, and arguments to the format!

As an escape hatch, any that require particularly custom handling could be declared as normal, and wouldn't need to use the DSL.

If you wanna see what the code looks like with all the current methods converted: https://github.com/blockfrost/blockfrost-rust/compare/master...SundaeSwap-finance:pi/pagination?expand=1

marcospb19 commented 2 years ago

Nice!

(I'm kinda convinced we are not gonna worry about filtering out query parameters (that second question), unexpected parameters are always ignored anyway.)

so you have the greenlight, PR is welcome :slightly_smiling_face:.

Quantumplation commented 2 years ago

Awesome; I'm gunna get some sleep now, but expect a PR tomorrow! (Also, Rust's macro system is unruly, but amazing... lol)

Quantumplation commented 2 years ago

(on a side note, while you're here... I know this isn't the right avenue for this, but how hard would it to get a /pool/{pool_id}/delegators/{epoch} endpoint, to find out the exact set of delegators for a given epoch snapshot in the past?)

marcospb19 commented 2 years ago

(on that side note, I'm not sure, I'll redirect the question to @mmahut and we'll answer you when possible :+1:)

mmahut commented 2 years ago

hey @Quantumplation! :)

I think you are looking for /epochs/{number}/stakes/{pool_id}. But it is true this endpoint would make also sense in the /pools list, we will add an alias to it there as well. Thank you!

Quantumplation commented 2 years ago

@mmahut Ohhhh, perfect! Yea, I totally missed that! Thank you! <3