0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
33 stars 29 forks source link

feat: add filter before storing P2ID and P2IDR notes #208

Closed mFragaBA closed 6 months ago

mFragaBA commented 7 months ago

addresses #131

When executing transactions, these transactions can create notes. If the notes are to be consumed by any of the accounts managed by the client, it makes sense to store them for execution (this means storing them in the input_notes table). If not, we can avoid storing them (we still store them in the output-notes table).

For now we'll handle P2ID and P2IDR notes as they're the only think we have. To check if we can consume them we just need to see that they have the corresponding script hash and the account id is the only note input.

I also implemented this as a function that belongs to the store trait which is implemented by default. This will make it more flexible for different stores to choose their own heuristic

I'd recommend merging this after #206 as there is introduced a test to ensure the note scripts are valid

mFragaBA commented 7 months ago

Thank you! I left some comments above - but overall, I think we need to rethink the overall approach to this problem to come up with a more comprehensive solution.

yeap, the two approaches thought so far were:

Are there any other ways of enusring a note might be consumable by an account?

bobbinth commented 7 months ago

Are there any other ways of enusring a note might be consumable by an account?

I think these are the approaches - but I don't think of them as mutually exclusive. Basically, I'm thinking of a component which given a note tells us if a given account can consume this note and when. This component has a "fast path" for standardized note scripts (such as P2ID and P2IDR) and a "slow path" for custom note scripts.

For custom note scripts we may also rely on some note metadata in the future to help with deciding if the note is applicable (we can build this in later).

Dominik1999 commented 6 months ago

Can we merge this today?

mFragaBA commented 6 months ago

Can we merge this today?

sorry for the delay, I think I've made the requested changes