Add Priority Scores #96
Conversation
|
JasonMHasperhoven
requested review from
grod220 and
conorsch
and removed request for
grod220
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@penumbra-labs/registry': patch | |||
There was a problem hiding this comment.
issue: this should be a minor bump given it is a forward-compatible new feature
tools/compiler/src/processor.rs
Outdated
| pb_metadata.priority_score = priority_scores_by_base | ||
| .get(&pb_metadata.base) | ||
| .copied() | ||
| .unwrap_or(1); |
There was a problem hiding this comment.
issue: if it is not in the HashMap, don't think we should add any score to the metadata object. Aka:
// Add priority score if available
if let Some(score) = priority_scores_by_base.get(&pb_metadata.base) {
pb_metadata.priority_score = *score;
}
tools/compiler/src/parser.rs
Outdated
| @@ -27,6 +28,7 @@ pub struct ChainConfig { | |||
| pub ibc_connections: Vec<IbcInput>, | |||
| pub native_assets: Vec<Metadata>, | |||
| pub canonical_numeraires: Vec<String>, | |||
| pub priority_scores_by_base: HashMap<String, u64>, | |||
There was a problem hiding this comment.
suggestion: Can you alias the string? Aka:
type BaseDenom = String
pub priority_scores_by_base: HashMap<BaseDenom, u64>,
input/chains/penumbra-1.json
Outdated
| "transfer/channel-4/factory/osmo1q77cw0mmlluxu0wr29fcdd0tdnh78gzhkvhe4n6ulal9qvrtu43qtd0nh8/crazyhorse": 1, | ||
| "transfer/channel-4/uosmo": 14, | ||
| "transfer/channel-4/factory/osmo10n8rv8npx870l69248hnp6djy6pll2yuzzn9x8/BADKID": 1, | ||
| "upenumbra": 99, |
There was a problem hiding this comment.
suggestion: can you move this to the top? Actually, can we rank these be priority scores descending?
There was a problem hiding this comment.
also, I'd make this 999999999999
There was a problem hiding this comment.
yeah, I think a good approach is:
- only specify overrides (if unspecified, will use a default score of 0)
- specify assets top to bottom
- leave spaces in the numbering (can put UM as 1000000, other stuff at 100000)
input/chains/penumbra-1.json
Outdated
| "transfer/channel-4/factory/osmo1pfyxruwvtwk00y8z06dh2lqjdj82ldvy74wzm3/WOSMO": 14, | ||
| "transfer/channel-4/factory/osmo19hdqma2mj0vnmgcxag6ytswjnr8a3y07q7e70p/wLIBRA": 1, | ||
| "transfer/channel-4/factory/osmo14mafhhp337yjj2aujplawz0tks6jd2lel4hkwz4agyzhvvztzaqsqzjq8x/alloyed/allTRX": 18, | ||
| "transfer/channel-4/factory/osmo1q77cw0mmlluxu0wr29fcdd0tdnh78gzhkvhe4n6ulal9qvrtu43qtd0nh8/cac": 1, |
There was a problem hiding this comment.
question: Is there a reason why we would add a priority score of 1 to any? Feels like we can drop it entirely if it's that low
As part of penumbra-zone/web#1829