feat: add trace offer for a series of content keys#1830
Conversation
| } | ||
|
|
||
| /// Send Offer request with trace, without storing the content into db, with multiple items | ||
| pub async fn send_offer_trace_with_multiple_items( |
There was a problem hiding this comment.
There is some duplicated logic between this and function above it (send_offer_trace).
Can't we make send_offer_trace simpler, something like this:
pub async fn send_offer_trace(...) -> Result<OfferTrace, OverlayRequestError> {
let content_items = vec![content_key, content_value];
match self.send_offer_trace_with_multiple_items(enr, content_items).await? {
OfferTraceMultipleItems::Success(accept_code_list) => {
...
}
OfferTraceMultipleItems::Failed => Ok(OfferTrace::Failed),
}
}| content_keys.len() | ||
| ); | ||
| if let Err(err) = tx.send(OfferTraceMultipleItems::Success(content_keys)) { | ||
| warn!(%err, "Unable to send OfferTrace Success result"); |
There was a problem hiding this comment.
If I'm not mistaken, tx.send would fail only if channel is closed, but we wouldn't know why it's closed.
And since all error messages are the same (except Success / Failed part), it would be hard to know in which case this happens.
I think it's probably not so important to even log these messages (how it was before), but if we already want to do it, I would describe them a bit better (e.g. they would include something that indicates which line actually printed the message). If you want, I can make suggestions for that.
Either way, no big deal. Up to you.
| if let Err(err) = tx.send(OfferTraceMultipleItems::Success(content_keys)) { | ||
| warn!(%err, "Unable to send OfferTrace Success result"); | ||
| } | ||
| let _ = tx.send(OfferTrace::Success(content_keys[0])); | ||
| } else { | ||
| let _ = tx.send(OfferTrace::Failed); | ||
| } else if let Err(err) = tx.send(OfferTraceMultipleItems::Failed) { | ||
| warn!(%err, "Unable to send OfferTrace Failed result"); |
There was a problem hiding this comment.
nit: I would do this slightly differently:
let result = if result {
OfferTraceMultipleItems::Success(content_keys)
} else {
OfferTraceMultipleItems::Failed
};
if let Err(err) = tx.send(result) {
warn!(%err, "Unable to send OfferTrace result");
}2 participants
What was wrong?
send_offer_traceonly accepts 1 key-value pair. In most cases that is fine, but for we want to send multiple key-value pairs in the Ephemeral History Bridge, so this doesn't work.How was it fixed?
I am open to suggestions. I think it is easier/cleaner to expose an api which allows for the 1 key-value pair case and the multiple key-value case as they are pretty distinct.