refactor: move upd_twap from upd_aggregate to rust upd_price #398
Conversation
cctdaniel
changed the title
| let agg_diff = { | ||
| (clock.slot as i64) | ||
| - load_checked::<PriceAccount>(price_account, cmd_args.header.version)?.last_slot_ | ||
| as i64 | ||
| }; | ||
|
|
There was a problem hiding this comment.
taken from upd_aggregate.h
|
|
||
| assert_eq!(price_data.agg_.price_, 100); | ||
| assert_eq!(price_data.agg_.conf_, 10); | ||
| assert_eq!(price_data.twap_.val_, 100); |
There was a problem hiding this comment.
Maybe better to keep the asserts and run c_upd_twap after c_upd_aggregate above
There was a problem hiding this comment.
hmm we already have a test_ema.rs which runs c_upd_twap do we need it here again?
guibescos
left a comment
guibescos
left a comment
There was a problem hiding this comment.
Would like to see other reviews
| let agg_diff = { | ||
| (clock.slot as i64) | ||
| - load_checked::<PriceAccount>(price_account, cmd_args.header.version)? | ||
| .prev_slot_ as i64 |
There was a problem hiding this comment.
so the prev_slot after c_upd_aggregate will remain the same as last_slotbefore calling c_upd_aggregate?
There was a problem hiding this comment.
yup since both prev_slot and last_slot is only updated in c_upd_aggregate if ptr->agg_.status_ == PC_STATUS_TRADING
|
|
||
| // get number of slots from last published valid price | ||
| int64_t agg_diff = ( int64_t )slot - ( int64_t )( ptr->last_slot_ ); | ||
|
|
There was a problem hiding this comment.
something to watch out for going forward is the if statement below this comment which updates the previous price. There's a chance that we introduce a bug that updates this multiple times on a single slot, which would make prev_slot == agg_.pub_slot_
this is the first refactor in part of the same slot agg changes, intentionally keeping it small and limited to
upd_twapso that it's easier to review