Closed evilrobot-01 closed 1 year ago
* Does `report.is_disputed = true;` get reversed and the value placed back in the series if execute vote determines the dispute fails or is invalid?
Forgive me if I have missed it, but I havent seen anywhere in the contracts where a disputed value is reversed. I just double checked the governance contract again and can only see it being removed, the disputed value being copied to the dispute and no restoration. Could you possibly provide a link if I have indeed missed it please?
* Could you provide some more explanation about this: `Includes the actual number of iterations done by the binary search so caller can be refunded based on weight difference.`? I'm not clearly understanding the workflow of the caller and the situations where they'd be doing this.
The binary search is used when looking up the timestamp index before some supplied timestamp, which may not match the actual timestamp of a submitted value. update_stake_amount
uses this twice, once for determining the price for the staking token query id and again for the other query id used for the dispute fee.
Each iteration of the binary search requires additional reads, which results in additional weight/cost. Based on the u32::MAX value as the maximum number of timestamps for a query id, the maximum iterations for a binary search will be 32, but depending on the number of timestamps which exist under a query id the number of iterations can vary. We benchmark to generate weight functions which can be called prior to dispatchable function execution (example) to ensure the caller has sufficient balance to cover the worst case fees and then called again at the end of the dispatchable function (example) to calculate actual weights used to have the caller refunded accordingly.
No action is required by the caller, its just gas metering ahead of time to generate a lightweight weight function (example) rather than the overhead of measuring in realtime, which is then callable with observed parameter values depending on use case. A simpler example is that of claiming timestamps, where claiming 100 will require more compute as opposed to 10 and we therefore need to charge accordingly.
For the first part, that's my mistake. Thanks for clarifying. Appreciate the further explanation of the number of binary search iterations and where it's used. Sounds good.
Optimises extrinsics and storage to read storage in a more efficient manner to reduce extrinsic weights.
A summary of changes:
Report
to track all info related to a timestamp which is generally read together, eliminating multiple reads which has an associated cost.previous
attribute, which tracks the previous undisputed timestamp before the report, which is maintained viaremove_value
. This eliminates the need to iterate over timestamps to find an undisputed value, as the 'linked list' is updated at the time of removal, optimising for reads especially as disputes are a somewhat rare event.get_index_for_data_before_with_start
'overload' which optimises functionality ofget_index_for_data_before
by using additional state to eliminate potentially excessive reads.start
index can be provided, which could be used to reduce the search range of oracle values (i.e. start at index of last valid oracle price rather than zero each time). This requires an additional storage item to track the index of the last used item, so would only make sense to start using once there are sufficient timestamps to make it worthwhile.get_index_for_data_before
can easily be updated to useget_index_for_data_before_with_start
once we are happyget_data_before_with_start
added, replicating functionality ofget_data_before
but using optimised function described abovedo_get_reward_amount
only reads reported values when actually requiredget_current_value_and_timestamp
as optimised awayget_onetime_tip_amount
uses optimised state to prevent searchremove_value
updates theprevious
attribute of subsequent timestamps, essentially maintaining a linked list as timestamps are disputed and removed, so that any timestamp always points to the previous undisputed timestamp. This effectively makes disputing potentially more expensive due to increased writes, but makes all other functions, such as those around tips, cheaper as there are no longer searches for timestamps.Reports<T>
as described above, used bysubmit_value
LastReportedTimestamp<T>
which tracks last undisputed timestamp per query_id (i.e. current value).MaxDisputedTimeSeries
config item, representing the maximum number of sequential disputed timestamps tolerated before erroring. This is only used inremove_value
.s
complexity parameter fromclaim_onetime_tip
,claim_tip
andtip
, which represented max submissions. No longer required due to usingReport.previous
; updated dispatchable functions accordinglys
andl
complexity parameters toupdate_stake_amount
to track number of iterations in binary search for two prices used; updated dispatchable function accordinglyd
complexity parameter tobegin_dispute
to track max number of sequential disputed timestamps before erroring; updated dispatchable function accordinglys
parameter tov
invote_on_multiple_disputes
andsend_votes
s
andl
complexity parameters toon_initialized
to track number of iterations in binary search for two prices used, along withv
for max votes sent per block; updatedon_initialize()
function accordingly to return actual values to weigh function.remove_values
test to test the various scenarios of timestamps being removed and the linked list being maintained accordinglyget_data_before
andget_index_for_data_before
to also call the new *with_start
overloads to ensure functionality is maintained with new implementationsget_index_for_data_before_with_start
test which compares performance with new function vs existingverify
test which ensures that all weight functions fall within block limits when using max valuesweights.rs
file after running benchmarksAdditional notes/observations:
begin_dispute
: subsequent dispute rounds are charged based on the benchmarked weights of the initial round only. This could be improved in the future by adding an additional benchmark to calculate the cost of a subsequent round and then update the weights returned accordingly.tip
: subsequent tips are charged at the cost of an initial tip. These are effectively equivalent apart from the additional write of the query data on the first.