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
conformance test 010 and 009 #45
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great with a few comments, the main thing being the message kind
change.
// | ||
// Current behaviour: | ||
// | ||
// zcashd: Ignores the following messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat expected as the node isn't seeded with any data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its.. inconsistent. I did initially seed it with blocks, and then GetBlocks
, and GetData(block)
work as expected.
Then one has GetData(tx)
which actually returns NotFound
despite not having data.
I would actually hope that all of these queries should result in an {empty}
style response, or NotFound
for the GetData
.
Essentially I think the node never sends an empty response except for GetData(tx)
- because the bitcoin protocol lists it as an example:
notfound is a response to a getdata, sent if any requested data items could not be relayed, for example, because the requested transaction was not in the memory pool or relay set.
In the end I opted to not seed anything until we can decide what is expected and correct. Happy to add the seeding back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to have both cases? This would also serve as documentation for the current behaviour in each case and make it easier for ECC/ZF to evaluate what the spec should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll split up the test case into seeded and unseeded
14a78f6
to
76e1063
Compare
555c2d3
to
661d138
Compare
I've changed to using a |
661d138
to
b77769c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good other than a few small things, love the macro 🎉
07a64ee
to
df183ff
Compare
df183ff
to
1e85a04
Compare
conformance test 010 and 009
Note that this includes the missing test messages for case 009.