trie.put fails with "Error: Stack underflow" #3645
Comments
|
Hi Tim! From reading the error messages, the issue seems to come from whatever is passed down to _updateNode. You are passing the key/value fields, but the keyRemained and stack are empty. Stack can't be empty in this context. I would recommend logging the stack upstream and seeing if that gives more clarity into the issue. |
For months now the reconciliation algo has been plagued by bugs surrounding the ethereumjs/trie library. We've opened many tickets on Github: - #146 - #131 - ethereumjs/ethereumjs-monorepo#3264 - ethereumjs/ethereumjs-monorepo#3645 The pattern of the problem was always that somehow that `trie.root()` couldn't be found using `trie.checkRoot`, which seemed almost like a contradiction, especially when doing `await trie.checkRoot(trie.root())`. We had initially introduced the checkpointing of the trie because of some rather theoretical problem regarding what would happen if during the reconciliation the trie updates and, at the same times, sends level comparisons to a peer. So to use checkpointing for us was primarily used to implement atomicity when storing data. We wanted to just store the remote trie's leaves in batches as to make sure not to interrupt the algorithm to compare the trie's levels. At the same time, the insertion of new leaves into such a trie is costly as a big part of its hashes have to be recomputed to arrive at a new root. However, I think what has happened with our implementation of the sync.put method is that the checkpointing led to the trie writes not being processed sequentially which also lead to all sorts of problems in the reconciliation. The reconciliation is purposefully built in a way where it first synchronizes old leaves and only then new leaves. While a working reconciliation doesn't have any issues with storing comments, a fundamentally asynchronous reconciliation will attempt to store comments where the original upvote hasn't been made yet, leading to the message not being processed initially. Another big problem ended up being that the ethereumjs/trie library isn't mature with regards to handling the application shutting down, and so a lot of the above mentioned issues actually describe the ethereumjs/trie library reaching a non-recoverable state. Funnily enough, however, all it took to fix all of the above problems was to remove all notions of checkpointing and commits. While it does make the reconciliation algorithm MUCH slower (because it is now synchronous), it also made it much more reliable and almost free of errors during interaction.
For months now the reconciliation algo has been plagued by bugs surrounding the ethereumjs/trie library. We've opened many tickets on Github: - #146 - #131 - ethereumjs/ethereumjs-monorepo#3264 - ethereumjs/ethereumjs-monorepo#3645 The pattern of the problem was always that somehow that `trie.root()` couldn't be found using `trie.checkRoot`, which seemed almost like a contradiction, especially when doing `await trie.checkRoot(trie.root())`. We had initially introduced the checkpointing of the trie because of some rather theoretical problem regarding what would happen if during the reconciliation the trie updates and, at the same times, sends level comparisons to a peer. So to use checkpointing for us was primarily used to implement atomicity when storing data. We wanted to just store the remote trie's leaves in batches as to make sure not to interrupt the algorithm to compare the trie's levels. At the same time, the insertion of new leaves into such a trie is costly as a big part of its hashes have to be recomputed to arrive at a new root. However, I think what has happened with our implementation of the sync.put method is that the checkpointing led to the trie writes not being processed sequentially which also lead to all sorts of problems in the reconciliation. The reconciliation is purposefully built in a way where it first synchronizes old leaves and only then new leaves. While a working reconciliation doesn't have any issues with storing comments, a fundamentally asynchronous reconciliation will attempt to store comments where the original upvote hasn't been made yet, leading to the message not being processed initially. Another big problem ended up being that the ethereumjs/trie library isn't mature with regards to handling the application shutting down, and so a lot of the above mentioned issues actually describe the ethereumjs/trie library reaching a non-recoverable state. Funnily enough, however, all it took to fix all of the above problems was to remove all notions of checkpointing and commits. While it does make the reconciliation algorithm MUCH slower (because it is now synchronous), it also made it much more reliable and almost free of errors during interaction.
|
Pretty sure revert, checkpoint and commit are faulty. I removed the entire logic now from my code base and I haven't run into these issues anymore. I'm pretty sure that if I create a checkpoint and add a ton of data and then my program panics without commit then there are issues with this library |
|
Hmm. We might want to add on the suite of test cases, that might be the most realistic way to catch that. 🤔 For trie itself we are not as good tested as for state manager (regarding the checkpointing logic). |
|
(can you guys nevertheless move over to v6? Or do you need to keep v5 for now?) |
|
We sadly don't have budget to move to v6 as of now. That code is mission critical and very costly to maintain because it is related to our set reconciliation algorithm. Not something I wanna mess with unless absolutely necessary, as it currently works, and since it is hard to get working in the first place. We run Kiwi News through self-funding etc., so we've been focusing as much as we can on making money to pay rent. I'm hopeful that we'll manage to upgrade some time in the future! |
Code: https://github.com/attestate/kiwistand/blob/7060bd6573f8b966d40e6af07d18d6ac2cb4a926/src/store.mjs#L282-L319
You can find the message in the logs and the index.
I'm using v5.0.4
It seems to be this line which is faulty:
ethereumjs-monorepo/packages/trie/src/trie/trie.ts
Line 377 in 295e5a3
I went in my ethereumjs/trie/dist/trie/trie.js and added a console log of all the function's inputs
this is what I get for the stack overflow throwing message
The text was updated successfully, but these errors were encountered: