feat(tenderdash-abci): deterministic request_id #38
Conversation
| let value = request.into(); | ||
| let value = request.value.as_ref().expect("request value is missing"); | ||
|
|
||
| // we use md5 as we need 16-byte request id for uuid, and it doesn't have to be |
There was a problem hiding this comment.
This approach has downsides, especially if we want to filter by specific requests in elastic. There are endpoints like info, query, etc. where we don't have much difference so they will have the same ID.
We could have both: unique request ID and md5 so we will benefit from both depending on what we need.
That would be great if we could use the same request ID with tenderdash so we can easily filter related events.
|
|
||
| // we use md5 as we need 16-byte request id for uuid, and it doesn't have to be | ||
| // cryptographically secure | ||
| let mut md5 = lhash::Md5::new(); |
There was a problem hiding this comment.
I think it's pretty expensive, especially for the process and prepare proposal that contain state transitions. Should we do that only if a specific logging level is enabled?
| // we use md5 as we need 16-byte request id for uuid, and it doesn't have to be | ||
| // cryptographically secure | ||
| let mut md5 = lhash::Md5::new(); | ||
| md5.update(&request.encode_to_vec()); |
There was a problem hiding this comment.
BTW, BLAKE3 much faster than md5 (which is not using in crypto world at all)
|
we decided it's not needed |
Issue being fixed or feature implemented
When analyzing logs, it would be useful if request ID on different nodes is the same.
Even if it means that request IDs are not unique.
What was done?
Request ID is derived from md5(request) instead of random.
How Has This Been Tested?
cargo testBreaking Changes
Request ID is not unique.
Checklist:
For repository code-owners and collaborators only