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

Create client side filtering spec #311

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cpacia
Copy link
Collaborator

@cpacia cpacia commented Apr 2, 2019

This is the spec for client side filtering as implemented in bchd. I'm submitting the PR here so that anyone looking to implement the spec in another client has a spec to base it on. Also, this spec isn't necessarily final and if there is feedback we are open to including it in bchd.

Specifically, it's unclear if the OP_RETURN additions are useful enough to justify keeping as part of this spec or not. Especially considering that they increase the filter size a little.

@cpacia cpacia force-pushed the master branch 3 times, most recently from 6a1a9f0 to 38a11b9 Compare April 2, 2019 03:23
@markblundeberg
Copy link
Collaborator

Is the resulting filter expected to be perfectly defined (i.e., every node gives same filter) ? If so then ExtractDataElements needs to be precisely defined taking into account that scripts may contain: weird push opcodes, non-push opcodes, or unparseable scripts that end mid-push (example).

@cpacia
Copy link
Collaborator Author

cpacia commented Apr 3, 2019

Is the resulting filter expected to be perfectly defined (i.e., every node gives same filter) ?

Yeah the expectation is every node calculates the filter the same because the lite clients will download both if they get two different ones from different nodes and ban the one that gave them the filter calculated differently from their code.

@markblundeberg
Copy link
Collaborator

markblundeberg commented Apr 3, 2019

Looks like that relies on what parseScript puts for pop.data with the special push opcodes -- for example, with OP_1. I suspect it's an empty array if it's like the other script parses I know (which is not the value that gets pushed on stack -- length 1 byte array [0x01]). That's totally fine it just has to be specified that way :-)

(Also, need to check parseScript to see just what it errors on -- pushes going past end of script, or other things too like using banned opcodes ...?)

@cpacia
Copy link
Collaborator Author

cpacia commented Apr 3, 2019

Yeah I'm looking at it. When I wrote the code I wasn't considering OP_0 - OP_16. Only OP_DATA_1 - OP_PUSHDATA4. And it looks like that's what I ended up with though the code could stand to be more explict.

Do you see any need to add OP_0 - OP_16 to the filter? Doesn't seem like any compelling use cases.

Also, the script parsing only fails if the pushdata opcodes have an invalid number of bytes in some way. And a parsing error causes the loop to continue without adding anything rather than adding the data elements that parsed correctly.

@markblundeberg
Copy link
Collaborator

Do you see any need to add OP_0 - OP_16 to the filter? Doesn't seem like any compelling use cases.

Indeed, I don't see any value there for flagging their inclusion. (BTW fun fact: OP_RESERVED is standard to use after OP_RETURN ... since it falls between OP_1 and OP_1NEGATE)

Also, the script parsing only fails if the pushdata opcodes have an invalid number of bytes in some way. And a parsing error causes the loop to continue without adding anything rather than adding the data elements that parsed correctly.

That sounds good. Those are nonstandard anyway btw, so they deserve punishment. :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants