Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend name_scan, obsolete name_filter #237

Closed
domob1812 opened this issue Jul 2, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@domob1812
Copy link

commented Jul 2, 2018

We currently have two different RPCs for reading the list of all (or a subset of) names: name_scan and name_filter. The main difference is that name_filter supports filtering by regexp and by age (number of confirmations).

I think we should unify the two methods. Based on #194, we can add an optional JSON options argument to name_scan which allows to specify desired filtering in a more flexible way. Then we can get rid of name_filter and resolve #16 at the same time with a new, specialised option to filter by namespace.

Here's the fields I propose for the new options argument to name_scan:

  • minConf: Minimum number of confirmations a name must have. Defaults to one. Zero (unconfirmed) is not allowed initially to keep things simple, but we may allow it later and obsolete (or complement) name_pending through that.
  • maxConf: Maximum number of confirmations to allow - defaults to no restriction if not set.
  • prefix: If set, only include names with this prefix (e.g. d/).
  • regexp: If set, only include names matching this regexp.
  • showExpired: If true, include expired names. Defaults to false. This could also be achieved with maxConf, but I think having an explicit option is cleaner, more explicit and easier to use.

One thing to note: Currently name_filter also supports a mode that does not return names but prints stats instead. I'm not sure if that is used at all (I've never used it), and that functionality would go away with this proposal. @JeremyRand and others: Do you think we need to keep this? If so, we could just add another returnStats boolean option.

@domob1812

This comment has been minimized.

Copy link
Author

commented Aug 11, 2018

@JeremyRand and all: What do you think about this proposal? Does it make sense, or did I miss some usage for name_filter that would not be supported by my proposed extension of name_scan?

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

@domob1812 I'd lean toward replacing namespace with prefix, and make the user supply d/ (with the forward-slash) rather than d (without the forward-slash). This makes the API a little bit more consistent with Namecoin's internal representation of names as binary blobs, and is also a bit more consistent with how people tend to write Namecoin namespaces (I usually see people refer to the d/ or id/ namespace, not the d or id namespace).

I didn't know there was a stats mode in name_filter; I've definitely never used it. I have no idea whether anyone is using it right now.

@domob1812

This comment has been minimized.

Copy link
Author

commented Aug 13, 2018

Yes, I agree that prefix instead of namespace is probably simpler and easier to understand. I've updated the OP accordingly.

No objections to removing name_filter when we add equivalent functionality (as per the proposal) to name_scan?

@domob1812 domob1812 added this to the nc0.18 milestone Sep 6, 2018

@domob1812 domob1812 self-assigned this Sep 6, 2018

@domob1812

This comment has been minimized.

Copy link
Author

commented Sep 6, 2018

It seems there are no more objections to this, so I'll go forward with it. This will also simplify the remaining parts of #246 after #261, because a) we then already have an options argument for name_scan, and b) name_filter will be gone.

domob1812 added a commit to domob1812/namecore that referenced this issue Oct 23, 2018

Support encoding options for name_scan.
This adds an "options" JSON object argument for name_scan, currently
supporting explicitly overriding the name and value encoding to be used
for the RPC.

This implements namecoin#246 for
name_scan, and will be the basis for implementing more options for
namecoin#237 in follow ups.

domob1812 added a commit to domob1812/namecore that referenced this issue Oct 23, 2018

Add filtering options for name_scan.
This extends the "options" argument for name_scan to allow filtering
of names by the number of confirmations, the prefix and a regexp as
described in namecoin#237.
("showExpired" is not yet implemented, as that will be part of a more
general change also for other read-only RPCs in a later step.)

With these options, name_scan now supports every feature of name_filter
except for the "stats" mode.  The latter will be removed in a follow-up
change; the name_scanning.py regtest is already updated to only test
name_scan (but all the new features) in this commit.

This also implements namecoin#16
through the new "prefix" option for name_scan.

domob1812 added a commit to domob1812/namecore that referenced this issue Oct 23, 2018

Remove name_filter.
In this commit, name_filter is removed completely.  The newly extended
name_scan (with filtering options) supports everything that name_filter
provided (except for the stats mode), so that we can simplify the RPC
interface and maintenance by having just one method for scanning through
the list of names.

Discussion: namecoin#237

domob1812 added a commit that referenced this issue Oct 27, 2018

Merge #269: Add filtering options to name_scan, making name_filter ob…
…solete

4a0a288 Remove name_filter. (Daniel Kraft)
695204e Add filtering options for name_scan. (Daniel Kraft)
e980bb9 Allow empty string with hex encoding. (Daniel Kraft)
8d3e8cb Support encoding options for name_scan. (Daniel Kraft)
e6097b8 Add _encoding / _error fields to help text. (Daniel Kraft)
fbbe194 Move help builder for "options" to rpc/names. (Daniel Kraft)
f156d78 Add options argument to getNameInfo. (Daniel Kraft)

Pull request description:

  In this change, we add a new optional `options` JSON object argument for `name_scan`.  It can be used to override the name and value encodings, implementing #246 for `name_scan`.  Furthermore, options specific to `name_scan` can be set to filter names based on the number of confirmations, their prefix or a regexp as discussed in #237.  (`showExpired` is not yet supported, as that will be added in a later step for other RPCs as well, as described in #194.)

  With this extension, `name_scan` now supports everything that `name_filter` does except for the stats mode.  Thus, `name_filter` is removed to simplify the RPC interface and reduce the future maintenance burden.  Users of `name_filter` should migrate to the new options of `name_scan`.

  This also finally implements the already closed feature request in #16.

Tree-SHA512: 1120f0f3ed24dc0c23346c8328cf2f865058118373674a8ee5d8f7233f16c23a3ba4feab5947666acc2ac394b5058b7bae16affb00d1272693ae4cb9abd61ddb

@domob1812 domob1812 closed this Oct 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.