Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Rpc: add filter to getProgramAccounts #10888

Merged
merged 8 commits into from
Jul 3, 2020

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Jul 2, 2020

Problem

Users will need a way to inspect and manage all the SPL Token accounts they own. This could be accomplished by calling getProgramAccounts for the token program-id and deserializing all the accounts to inspect the owner field. But spl-token::State is not yet ready/published to be pulled in as a dependency, and moreover that deserialization is a lot of overhead.
Instead, we can use a generic byte-comparison filter. This gives users flexibility as to what part of account data is matched, and gives us the opportunity to add different types of filters in the future.

Summary of Changes

  • Add RpcFilterType and implement a byte-comparison filter for getProgramAccounts
    • Filters should be passed as part of the config struct; an array of filters is accepted, and all must pass in order for an account to be included in the response
    • The memcmp filter object looks like this: {"memcmp":{"offset":44,"bytes":"7kgfDmgbEfypBujqn4tyApjf8H7ZWuaL3F6Ah9vQHzgR"}}, where offset is the offset into account.data that will be matched, and bytes are base-58 encoded string representation of bytes to compare

TODO:

  • update docs
  • update rpc tests

Sorry, something went wrong.

@CriesofCarrots
Copy link
Contributor Author

@mvines , how does this direction look?
It occurred to me just now that I could strip out all the Rpc-specific stuff from the filter module and move it into solana-sdk. Wdyt?
Also, thoughts on passing bytes in base-58? In addition to being more consistent, it's rather nice for pubkey matching. Eg. this is finding a stake account by withdrawer address:

curl -X POST -H "Content-Type: application/json" -d '{"jsonrpc":"2.0", "id":1, "method":"getProgramAccounts","params":["Stake11111111111111111111111111111111111111",{"filters":[{"compareBytes":{"offset":44,"bytes":"7kgfDmgbEfypBujqn4tyApjf8H7ZWuaL3F6Ah9vQHzgR"}}]}]}' 127.0.0.1:8899

{"jsonrpc":"2.0","result":[{"account":{"data":"PRkgDHKba5bHL9XzCoLPcZZSNZduEZ8Nfy1ttFrzzSUUR85CinoYnEdDbHMS36W22GwE8P1qgVBh3RmeEZ4E2cazGmw9MA6EHDvmcWbxRseYhUoErt45A5KZ5oCN2Wx1Hemvs97NbEbLrVYHSkBUcuFht4DgmgaaoZ5ou3KNYeMEkwhbp3NHTfxkAmUTjasLBfTVz4ewbi5HqABLs3dUhy3eMXU9QVUT9j4yfbdL1fMLv6FfW4DNghFxNvczm5kCUk5jsFxhAFi4MkTZ","executable":false,"lamports":153333632445409120,"owner":"Stake11111111111111111111111111111111111111","rentEpoch":0},"pubkey":"13LeFbG6m2EP1fqCj9k66fcXsoTHMMtgr7c78AivUrYD"}],"id":1}

@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #10888 into master will increase coverage by 0.0%.
The diff coverage is 90.6%.

@@           Coverage Diff            @@
##           master   #10888    +/-   ##
========================================
  Coverage    81.8%    81.9%            
========================================
  Files         308      309     +1     
  Lines       71447    71602   +155     
========================================
+ Hits        58511    58655   +144     
- Misses      12936    12947    +11     

@mvines
Copy link
Contributor

mvines commented Jul 2, 2020

@mvines , how does this direction look?
It occurred to me just now that I could strip out all the Rpc-specific stuff from the filter module and move it into solana-sdk. Wdyt?

Is there a non-Rpc consumer you had in mind for it?

Also, thoughts on passing bytes in base-58? In addition to being more consistent, it's rather nice for pubkey matching.

Yeah that's probably best for now (slowness of base58 aside). Although we could establish a precedent for other encoding types by including an optional "encoding" field with encoding: "binary" as the default (🤢 , but we've been using "binary" throughout the RPC API to mean base58 so I think we're fully committed at this point).

The C developer in me wants to say memcmp instead of compareBytes :)

A "dataSize" filter would also be immediately useful too, with spl-token going in the direction of the multisig account size being different than the rest.

I can specific two compareBytes filters right? I might want to compare the first byte of the account data to select the subaccount type, then look for that pubkey at offset 44.

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented Jul 2, 2020

Is there a non-Rpc consumer you had in mind for it?

No, nothing specific at present. There's just nothing intrinsically rpc about the filter.

Yeah that's probably best for now (slowness of base58 aside). Although we could establish a precedent for other encoding types by including an optional "encoding" field with encoding: "binary" as the default (🤢 , but we've been using "binary" throughout the RPC API to mean base58 so I think we're fully committed at this point).

Sure. Yeah, I have regrets about that "binary". It might not too difficult to deprecate that and call it "base58" instead, at some point.

The C developer in me wants to say memcmp instead of compareBytes :)

I'm fine with that

A "dataSize" filter would also be immediately useful too, with spl-token going in the direction of the multisig account size being different than the rest.

Cool, I'll roll that in.

I can specific two compareBytes filters right? I might want to compare the first byte of the account data to select the subaccount type, then look for that pubkey at offset 44.

Yep, currently using AND logic between all filters. We could probably support AND NOT cases really easily, but supporting ORs as well would be a can of worms.

@mvines
Copy link
Contributor

mvines commented Jul 2, 2020

Is there a non-Rpc consumer you had in mind for it?

No, nothing specific at present. There's just nothing intrinsically rpc about the filter.

I think I'd keep it Rpc-specific for now. Generalize when we have another user

@CriesofCarrots CriesofCarrots force-pushed the accounts-filter branch 2 times, most recently from 3d235a4 to 90914aa Compare July 2, 2020 22:06
@CriesofCarrots CriesofCarrots marked this pull request as ready for review July 2, 2020 22:06
@CriesofCarrots
Copy link
Contributor Author

@mvines , this is ready for another look.
Regarding the dataSize filter, I considered supporting different comparators (gt, lt, ne, etc), but ultimately went with a simple equality check. Let me know if you think the greater flexibility seems needed right now.

mvines
mvines previously approved these changes Jul 3, 2020
Copy link
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to see a test with two filters: two memcmps, memcmp and dataSize. Looks great

@mergify mergify bot dismissed mvines’s stale review July 3, 2020 06:29

Pull request has been modified.

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Jul 3, 2020
@mergify mergify bot merged commit 8d95177 into solana-labs:master Jul 3, 2020
CriesofCarrots added a commit to CriesofCarrots/solana that referenced this pull request Jul 6, 2020
* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
CriesofCarrots added a commit to CriesofCarrots/solana that referenced this pull request Jul 6, 2020
* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
mergify bot pushed a commit that referenced this pull request Jul 6, 2020
* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
CriesofCarrots pushed a commit to CriesofCarrots/solana that referenced this pull request Jul 7, 2020
CriesofCarrots added a commit to CriesofCarrots/solana that referenced this pull request Jul 7, 2020
* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
CriesofCarrots added a commit to CriesofCarrots/solana that referenced this pull request Jul 7, 2020
* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
CriesofCarrots added a commit that referenced this pull request Jul 7, 2020
* Revert "Rpc: add filter to getProgramAccounts (#10888) (#10932)"

This reverts commit 9311a6e.

* Add jsonParsed option for EncodedTransactions; add memo parser (#10711)

* Add jsonParsed option for EncodedTransactions; add memo parser

* Use kebab case for program names

* Add account-key parsing

* Add parse test

* Update transaction encoding docs (#10833)

* Add account-decoder utilities (#10846)

* Fix comment and make less pub

* Add account-decoder crate and use to decode vote and system (nonce) accounts

* Update docs

* Rename RpcAccount struct

* s/Rpc/Display

* Call it jsonParsed and update docs

* Revert "s/Rpc/Display"

This reverts commit 6e7149f.

* s/Rpc/Ui

* Add tests

* Ui more things

* Comments

* Update struct prefixes to Ui (#10874)

* Update comments

* Use Ui prefix

* Rpc: add filter to getProgramAccounts (#10888)

* Add RpcFilterType, and implement CompareBytes for getProgramAccounts

* Accept bytes in bs58

* Rename to memcmp

* Add Memcmp optional encoding field

* Add dataSize filter

* Update docs

* Clippy

* Simplify tests that don't need to test account contents; add multiple-filter tests
@CriesofCarrots CriesofCarrots deleted the accounts-filter branch July 24, 2020 22:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants