Skip to content

Commit

Permalink
[rpc] Fix wrong assumption about event digests (MystenLabs#9915)
Browse files Browse the repository at this point in the history
## Description 

Multiple tx share the same event could happen, because event doesn't
have id or timestamp, if the content is the same they will have the same
digest

## Test Plan 

Build

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
666lcz authored Mar 27, 2023
1 parent 585bc7e commit f6d7e16
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions crates/sui-json-rpc/src/read_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use move_bytecode_utils::module_cache::GetModule;
use move_core_types::language_storage::StructTag;
use move_core_types::value::{MoveStruct, MoveStructLayout, MoveValue};
use tap::TapFallible;
use tracing::debug;
use tracing::{debug, error};

use shared_crypto::intent::{AppId, Intent, IntentMessage, IntentScope, IntentVersion};
use sui_core::authority::AuthorityState;
Expand Down Expand Up @@ -493,35 +493,43 @@ impl ReadApiServer for ReadApi {
let events = self
.state
.multi_get_events(&event_digests_list)
.map_err(|e| anyhow!("{e}"))?
.map_err(|e| {
error!("Failed to call multi_get_events for transactions {digests:?} with event digests {event_digests_list:?}");
anyhow!("{e}")
})?
.into_iter();

// construct a hashmap of event digests -> events for fast lookup
let mut event_digest_to_events = event_digests_list
let event_digest_to_events = event_digests_list
.into_iter()
.zip(events)
.collect::<HashMap<_, _>>();

// fill cache with the events
for (_, cache_entry) in temp_response.iter_mut() {
let transaction_digest = cache_entry.digest;
let event_digest: Option<Option<TransactionEventsDigest>> = cache_entry
.effects
.as_ref()
.map(|e| e.events_digest().cloned());
let event_digest = event_digest.flatten();
if event_digest.is_some() {
// safe to unwrap because `is_some` is checked
let event_digest = event_digest.as_ref().unwrap();
let events: Option<RpcResult<SuiTransactionEvents>> = event_digest_to_events
.remove(event_digest.as_ref().unwrap())
.expect("This can only happen if there are two or more transaction digests sharing the same event digests, which should never happen")
.map(|e| to_sui_transaction_events(self, cache_entry.digest, e));
.get(event_digest)
.cloned()
.unwrap_or_else(|| panic!("Expect event digest {event_digest:?} to be found in cache for transaction {transaction_digest}"))
.map(|events| to_sui_transaction_events(self, cache_entry.digest, events));
match events {
Some(Ok(e)) => cache_entry.events = Some(e),
Some(Err(e)) => cache_entry.errors.push(e.to_string()),
None => cache_entry.errors.push(format!(
"Failed to fetch events with event digest {:?}",
event_digest.unwrap()
)),
None => {
error!("Failed to fetch events with event digest {event_digest:?} for txn {transaction_digest}");
cache_entry.errors.push(format!(
"Failed to fetch events with event digest {event_digest:?}",
))
}
}
} else {
// events field will be Some if and only if `show_events` is true and
Expand Down

0 comments on commit f6d7e16

Please sign in to comment.