Drozdziak1/permissioned price accounts #65
Conversation
drozdziak1
changed the title
drozdziak1
changed the title
src/agent/solana/exporter.rs
Outdated
| // Update permissioned accounts of this publisher from the | ||
| // oracle. The loop ensures that we clear the channel and use | ||
| // only the final, latest message; try_recv() is non-blocking. | ||
| loop { |
There was a problem hiding this comment.
I think it would be better to put this in run above? I think you can probably set up the publish_interval timer such that this processing happens between the publish_updates calls and doesn't interfere with them.
(also would be better from a code clarity perspective to put this in a separate function like flush_perm_price_channel() and call it from run())
There was a problem hiding this comment.
oh also you may want to look into watch::Receiver thing for NetworkState above, which seems to do exactly the thing you're doing here.
There was a problem hiding this comment.
Exiled this block to a helper method. I got into some nasty Rust errors when trying to use the watch channel. I explain this in detail in the new update_our_prices method doc, but it boils down to this:
Rustc claims the channel's internal RwLockReadGuard is not Send. This does not sound right, because all generics in tokio::sync::RwLockReadGuard<HashMap<Pubkey, HashSet<Pubkey>>> should meet that requirement. All the enclosing types are Send if the contents type is. Pubkey is Send and happens to be the contents.
I wouldn't put this call inside run, because we would either need to duplicate the keypair request code again, or do it in run. I think the intention with run is to just say "we're doing this one thing periodically".
| false | ||
| } | ||
| }) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
isn't this going to generate a ton of warnings in normal usage? If every publisher has different permissions on mainnet and pythnet, then I think fresh_updates here will contain the union of both. Which means that every time this code runs for solana mainnet, they will get a lot of warnings for the unpermissioned symbols.
src/agent/solana/exporter.rs
Outdated
| perm_price_rx: mpsc::Receiver<HashSet<Pubkey>>, | ||
|
|
||
| /// Symbols we are permissioned to publish | ||
| perm_price_accounts: HashSet<Pubkey>, |
There was a problem hiding this comment.
maybe these permissions should go in NetworkState? I think that already contains network-specific information, which this is also.
There was a problem hiding this comment.
eh... maybe not. i see why you added this channel, because you want to get the information from the Oracle, which is already getting the other price account information. Maybe this is fine.
This change prevents the agent from publishing prices that are not permissioned to its publishing keypair, potentially limiting the burn rate of the publishers' production deployments. The invalid attempts are logged to WARN.
How this works
*
solana.rs- we open a channel between oracle and exporter, periodically updating the latest known set of permissioned price accounts.oracle.rs- In addition to on-chain state lookup, we filter out permissioned prices and send them over the channel to the exporter. This is done by checking thePriceComp.publisherfield against the publish keypair from key store.exporter.rs- Just before building price update Solana transactions, we check for permissioned price set updates on the channel. If no updates are available, pre-existing value from exporter state is used. We filter out unpermissioned symbols using this set and log them to WARN. We continue with publishing the remaining valid symbols.Testing
Review highlights
Remaining items