add support for unallocated IOT rewards #675
Conversation
andymck
force-pushed
the
andymck/iot-unallocated-rewards-support
branch
from
51e9c25 to
d7b9a3e
Compare
andymck
force-pushed
the
andymck/iot-unallocated-rewards-support
branch
from
46cbd95 to
cb9ccd0
Compare
andymck
force-pushed
the
andymck/iot-unallocated-rewards-support
branch
from
a5f6ea3 to
f53ae67
Compare
andymck
force-pushed
the
andymck/iot-unallocated-rewards-support
branch
from
d385a8d to
6749669
Compare
iot_verifier/src/reward_share.rs
Outdated
| tracing::info!( | ||
| %total_dc_shares, | ||
| %total_dc_transfer_rewards_used, | ||
| %dc_transfer_rewards_unused, | ||
| %dc_transfer_rewards_per_share, | ||
| "data transfer rewards" | ||
| ); | ||
| self.beacon_rewards_per_share = beacon_rewards_per_share; |
There was a problem hiding this comment.
Are these values defaulted before this function is called? If so, this function should be a constructor, not a &mut function. Could be a pretty dangerous footgun
There was a problem hiding this comment.
Addressed in sep PR via this commit: 79925fa
iot_verifier/src/reward_share.rs
Outdated
| @@ -898,14 +934,22 @@ mod test { | |||
| reward_shares_in_dec(dec!(150), dec!(350), gw6_dc_spend), | |||
| ); // 0.0150, 0.0350 | |||
|
|
|||
| let gw_shares = GatewayShares { shares }; | |||
| let mut gw_shares = GatewayShares { | |||
There was a problem hiding this comment.
Instead of doing this, construct a new set of gateway shares from an iterator of hashmaps
There was a problem hiding this comment.
Addressed in sep PR via this commit: 79925fa
iot_verifier/src/reward_share.rs
Outdated
| + gateway_reward.beacon_amount | ||
| + gateway_reward.witness_amount; | ||
| ( | ||
| total_gateway_reward, |
There was a problem hiding this comment.
does including this total serve any purpose other than testing? wouldn't it be cleaner to just grab the reward message and sum the three reward categories in the test if that's the only place it's used?
There was a problem hiding this comment.
this is used in the rewarder, to accumulate the final u64 value of each gateway reward and from this to work out the final unallocated reward amount
…upport-refactor refactor and address review comments
There was a problem hiding this comment.
approved. i still think at a later time i'd like to see the way we calculate total allocated rewards more closely mirror how we calculate the total_shares() method on GatewayRewardShares, i.e. all at one time and independently of other operations in the same method since the into_iot_reward_shares method now seems a bit overloaded computing a total decimal of bones, converting the reward shares to the proto type and writing them to the file sink client.
Note: |
Linked proto PR: helium/proto#384