Add pool type filtering to GetMempoolTx#11
Conversation
…may prune" to "must prune".
`service.Exclude` has been renamed to `service.GetMempoolTxRequest` and has an added `poolTypes` field, which allows the caller of this method to specify which pools the resulting `CompactTx` values should contain data for.
pacu
left a comment
pacu
left a comment
There was a problem hiding this comment.
Approved modulo documentation suggestions.
Co-authored-by: Pacu <francisco.gindre@gmail.com>
nuttycom
force-pushed
the
feature/get_mempool_tx_pools
branch
from
1ee29f6 to
65eef23
Compare
Co-authored-by: Jack Grigg <jack@electriccoin.co>
nuttycom
force-pushed
the
feature/get_mempool_tx_pools
branch
from
65eef23 to
7c130e8
Compare
| // A list of transaction ID byte string suffixes that should be excluded | ||
| // from the response. These suffixes may be produced either directly from | ||
| // the underlying txid bytes, or, if the source values are encoded txid | ||
| // strings, by truncating the hexadecimal representation of each | ||
| // transaction ID to an even number of characters, and then hex-decoding | ||
| // and then byte-reversing this value to obtain the byte representation. |
There was a problem hiding this comment.
Do I understand this correctly as allowing either raw byte values OR a hex string? Would the server (lightwalletd) try interpreting the bytes both ways? If so, I don't think I like that; why not just require raw bytes? The types would match better too; a hex string should be type "string" not "bytes".
I think the problem str4d identified is that raw bytes should always be little-endian. What I was going to suggest is to rename this field (index 1) to something like exclude_txid_suffixes_bendian_deprecated and then have a new index, let's say 2, called exclude_txid_suffixes, which orders the bytes correctly (little-endian). After some time of supporting both, we can drop the deprecated one.
There was a problem hiding this comment.
Sorry, I wrote the above comment on a non-refreshed version of this page; I'm not sure if any of it applies. I'll take a look at the other comments and let you know if I have any suggestions.
There was a problem hiding this comment.
No, this is always txid bytes; what is described is just two different ways of obtaining the suffix.
And again, these bytes are not big-endian or little-endian; they are hashes, which don't have endianness. The fact that they were represented as uint256 values internal to zcashd was an unfortunate artifact of bitcoin history.
There was a problem hiding this comment.
And to clarify, we confirmed that this field is already used with bytes in the correct order; the wording of this comment is intended to precisely say how you get to those correctly-ordered bytes. It's just which end of the hash the bytes are truncated towards that is weird.
| repeated bytes exclude_txid_suffixes = 1; | ||
| // We reserve field number 2 for a potential future `exclude_txid_prefixes` | ||
| // field. | ||
| reserved 2; |
There was a problem hiding this comment.
I don't think we will ever need to support this, since a suffix is just as random as a prefix, but I suppose it's okay to reserve it.
There was a problem hiding this comment.
The reason we might want to work via prefixes is that it's natural to use finger trees as the indexing structure for transactions in the mempool, and using a prefix there would make more sense than a suffix.
4 participants
closes #10