l1-filter: Add TxFilterConfig #458
Conversation
| /// Addresses that are spent to | ||
| SpentToAddrs(Vec<BitcoinAddress>), | ||
| /// Blob ids that are expected | ||
| BlobIds(Vec<Buf32>), |
There was a problem hiding this comment.
These vecs can be sorted and then we can do binary search.
Having Vec instead of Individual items lets us group the items, sort and do binary search as we want.
There was a problem hiding this comment.
It doesn't make sense to have a list of rules where there there's multiple entries of the same variant. And in most cases we expect that all variants will end up being present exactly once. The ticket specified using a struct for this instead of a list of enum instances, and we'd do the filtering based on that directly.
| block: &Block, | ||
| filter_config: TxFilterConfig, | ||
| ) -> Vec<ProtocolOpTxRef> { | ||
| let filter_rules = filter_config.into_rules(); |
There was a problem hiding this comment.
The config is converted to existing rules with added variants. We can add complex logic here if needed. But this should probably suffice.
| /// Addresses that are spent to | ||
| SpentToAddrs(Vec<BitcoinAddress>), | ||
| /// Blob ids that are expected | ||
| BlobIds(Vec<Buf32>), |
There was a problem hiding this comment.
It doesn't make sense to have a list of rules where there there's multiple entries of the same variant. And in most cases we expect that all variants will end up being present exactly once. The ticket specified using a struct for this instead of a list of enum instances, and we'd do the filtering based on that directly.
| /// For deposits that might be spent from. | ||
| expected_outpoints: Vec<(Buf32, u32)>, |
There was a problem hiding this comment.
Should use a dedicated type for this.
| // For bridge V1 deposits that do bitmap flagging for the multisig addr. | ||
| // operator_key_tbl: PublickeyTable, | ||
| /// EE addr length | ||
| ee_addr_len: u8, |
| /// Deposit denomination | ||
| deposit_denomination: u64, // sats |
There was a problem hiding this comment.
Don't bake this in as a single value, it should be capable of being extended to support multiple denominations easily even if we don't support this today.
| /// Operators addr | ||
| operator_addr: BitcoinAddress, |
There was a problem hiding this comment.
Possibly redundant, this would be part of expected_addrs.
| pub fn from_rollup_params(rollup_params: &RollupParams) -> anyhow::Result<Self> { | ||
| let operator_wallet_pks = get_operator_wallet_pks(rollup_params); | ||
| let address = generate_taproot_address(&operator_wallet_pks, rollup_params.network)?; | ||
|
|
There was a problem hiding this comment.
We will be be computing the rules based on the last checkpoint's bridge state and the rollup params. I'm not sure this function should be on this type.
| tx: &Transaction, | ||
| filters: &[TxFilterRule], | ||
| ) -> Option<ProtocolOperation> { | ||
| fn extract_protocol_op(tx: &Transaction, filters: &[TxFilterRule]) -> Option<ProtocolOperation> { |
There was a problem hiding this comment.
Rework this to operate with the config directly.
Description
Type of Change
Checklist
Related Issues