feat: re-enable relevant record pricing as the basis for cost/storage #1497
Conversation
joshuef
force-pushed
the
RelevantPricing
branch
from
2d1c099
to
082cc50
Compare
joshuef
force-pushed
the
RelevantPricing
branch
from
082cc50
to
88012a3
Compare
joshuef
force-pushed
the
RelevantPricing
branch
from
88012a3
to
43f9545
Compare
There was a problem hiding this comment.
I'd suggest to split this PR into three:
1, lower the exponential pricing trigger to be 60%
2, remove payment_received times factor in pricing
3, re-introduce relevant records calculation
so that they can be better discussed/approved and merged better and quicker
|
@maqi I appreciate that separating things out might clarify each, but I'm not clear what's contentious in those parts? for me:
This has to go as is subjective to the node and cannot be verified. (It was added as a simple way to get nodes paid sooner, but it plays with pricing in odd ways, so best to get rid).
This is probably the most controversial? It's a simple change we'll need to evaluate though, so I wouldn't be shy about adding it back in, myself. and |
| self.received_payment_count); | ||
| NanoTokens::from(cost) | ||
| } | ||
| let record_keys_as_hashset: HashSet<Key> = self.records.keys().cloned().collect(); |
There was a problem hiding this comment.
I think we can prevent a clone here if we just collect the &Key. You might have to modify get_records_within_distance_range to accept references of keys.
| /// Notify the node received a payment. | ||
| pub(crate) fn payment_received(&mut self) { | ||
| self.received_payment_count = self.received_payment_count.saturating_add(1); | ||
| if let Some(distance_range) = self.distance_range { |
There was a problem hiding this comment.
I think the distance_range is not set until we have reached the max num of records inside prune_storage_if_needed_for_record. So think we might have to tweak how we set the distance_range value.
Description
reviewpad:summary