Dev wasm executor panics on block production #11132
Comments
ggwpez
added
I5-tests
|
I can take a look at this if you want, if you can provide the Python script necessary to reproduce this. |
|
Thanks @koute, here it is killer.py.txt (+ Just to recap:
|
|
Stack trace from the runtime (after my two PRs it now gets printed out): So it calls this function from /// Implementation for the `candidate_events` function of the runtime API.
// NOTE: this runs without block initialization, as it accesses events.
// this means it can run in a different session than other runtime APIs at the same block.
pub fn candidate_events<T, F>(extract_event: F) -> Vec<CandidateEvent<T::Hash>>
where
T: initializer::Config,
F: Fn(<T as frame_system::Config>::Event) -> Option<inclusion::Event<T>>,
{
use inclusion::Event as RawEvent;
<frame_system::Pallet<T>>::read_events_no_consensus()
.into_iter()
.filter_map(|record| extract_event(record.event))
.map(|event| match event {
RawEvent::<T>::CandidateBacked(c, h, core, group) =>
CandidateEvent::CandidateBacked(c, h, core, group),
RawEvent::<T>::CandidateIncluded(c, h, core, group) =>
CandidateEvent::CandidateIncluded(c, h, core, group),
RawEvent::<T>::CandidateTimedOut(c, h, core) =>
CandidateEvent::CandidateTimedOut(c, h, core),
RawEvent::<T>::__Ignore(_, _) => unreachable!("__Ignore cannot be used"),
})
.collect()
}which in turn calls this from /// Get the current events deposited by the runtime.
///
/// Should only be called if you know what you are doing and outside of the runtime block
/// execution else it can have a large impact on the PoV size of a block.
pub fn read_events_no_consensus() -> Vec<EventRecord<T::Event, T::Hash>> {
Events::<T>::get()
}So it looks like it tries to decode this value, and it's too big, so it can't allocate a big enough Curiously, we can see this comment where it is defined: /// Events deposited for the current block.
///
/// NOTE: This storage item is explicitly unbounded since it is never intended to be read
/// from within the runtime.
#[pallet::storage]
pub(super) type Events<T: Config> =
StorageValue<_, Vec<EventRecord<T::Event, T::Hash>>, ValueQuery>;"it is never intended to be read from within the runtime" and yet it is? (Unless "by runtime" here means "during normal block execution"?) Some potential ways we could fix this:
Alas it's outside of my area of expertise to be able to tell what's the most appropriate way to fix this. |
ggwpez
added
I3-bug
|
This is not really a surprise. When you shove a million transfers into one (or few) blocks, then you are going to have millions of events deposited (I think we deposit like 4 events per transfer), and reading them will cause the runtime to crash. Interestingly, because we As the name suggests, events should not, exactly for this reason, be read in the runtime. I am not why and how it is done now in the parachains runtime, but it is def. something to be reconsidered. cc @ordian @rphmeier. |
|
UPDATE: this code is reading events in a runtime api call, not in a consensus-context runtime execution code. So while someone should look at the implications of this runtime api potentially failing, this has no severe consensus issues. |
I think I would go with this. Having some kind of special interface However, before we do this we should check the size of the Polkadot runtime level Event with @ggwpez could you get us the size with |
@bkchr I assume Someone go 🥊 this. PS: |
You also need to take into account the allocation strategy. They are probably always increasing by 2 or something when they hit the upper size of the vector. Otherwise you are running constantly into allocations. So, I would assume it is more around 34-40k events. |
|
I have brought this up multiple times but can we just not having all the events stored in a single storage key? It is not light client / XCM friendly anyway. No one is using event topics but we should be using it. The only way to prove something have happened on other chain is via events. But we don't really want to send all the events in a single block just to prove a single event. |
|
Well, we could also optimize this on the parachains runtime side by not using FRAME events for |
|
This is one of the previous discussion #9480 (comment) I don't see a simple solution to this so we really need to spend sometime to think about it and come up some potential solutions for further discussion. I asked Gav about it last year May and what I got is "we may refactor the events system with a new cryptographic structure in the not too distant future." |
I try to benchmark something and the wasm executor seems to panic for the
polkadot-devruntime by hitting a memory limit.Failed to allocate memory: "Requested allocation size is too large"andgoing to fail due to allocating 66774016.Following setup:
build-spec --chain=polkadot-devpolkadot --chain dot-spec-1M-accs.json --base-path /tmp/dot-dev --execution wasm --wasm-execution compiled --pruning archive --force-authoring --alice --pool-limit 100000//Aliceto//Bob/i), I can provide the python script if needed.You should see this:
The text was updated successfully, but these errors were encountered: