oracle-core
oracle-core copied to clipboard
[100 SigmaUSD] Newtypes for epoch id/counter, datapoint, height
Motivation
To not confuse them in function parameters with each other and with other integer values.
Implementation details
Use the newtypes in the relevant structs, function parameters, etc. Underneath, epoch id/counter should be u32, datapoint - i64, height - u32.
I wonder whether something like a builder pattern would be better for this.
So calls to this for example
#[allow(clippy::too_many_arguments)]
pub fn build_refresh_action(
pool_box_source: &dyn PoolBoxSource,
refresh_box_source: &dyn RefreshBoxSource,
datapoint_stage_src: &dyn DatapointBoxesSource,
max_deviation_percent: u32,
min_data_points: u32,
wallet: &dyn WalletDataSource,
height: u32,
change_address: Address,
my_oracle_pk: &EcPoint,
)
could be turned into something along these lines:
let refresh_action = RefreshActionBuilder::new()
.with_pool_box_source(&pool_box)
.with_refresh_box_source(&update_box)
.with_max_deviation_percent(deviation_percent)
.with_height(height)
.build()?
That way it's easier to verify for the caller to see if the right parameters were passed without having to wrap parameters in newtypes
@SethDusek I'm not sure a builder pattern will be a better fit here. It shines when some of the parameters are optional, which is not our case (we need all of them anyway). In terms of checking the parameter types, it does help the human eye by providing a kind of "parameter names" to catch on. But nothing beats a compiler error in preventing from passing an epoch id instead of height. :)
That's a good point yeah, there aren't really any optional parameters as well. I guess newtypes are the better fit here
I will work on this
The bounty is sent. Thank you!