Skip to content

Commit aab8773

Browse files
authored
fix: native world state test issues (#8546)
This PR fixes the flaky Jest tests that were comparing the C++ WorldState against the JS implementation. The issue stemmed from the `WorldState::get_state_reference` function reading state reference for all five trees in parallel and saving that information in a `std::unordered_map`. Concurrent writes to a `std::unordered_map` [are invalid](https://devblogs.microsoft.com/oldnewthing/20231103-00/?p=108966) so I've added a lock to ensure that only one thread writes at a time. Simultaneous reads are fine though.
1 parent 578f67c commit aab8773

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

barretenberg/cpp/src/barretenberg/world_state/world_state.cpp

+11-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "barretenberg/world_state/tree_with_store.hpp"
1010
#include "barretenberg/world_state/types.hpp"
1111
#include <memory>
12+
#include <mutex>
1213
#include <stdexcept>
1314
#include <tuple>
1415
#include <unordered_map>
@@ -96,18 +97,19 @@ StateReference WorldState::get_state_reference(WorldStateRevision revision) cons
9697
{
9798
Signal signal(static_cast<uint32_t>(_trees.size()));
9899
StateReference state_reference;
100+
// multiple threads want to write to state_reference
101+
std::mutex state_ref_mutex;
102+
99103
bool uncommitted = include_uncommitted(revision);
100104

101105
for (const auto& [id, tree] : _trees) {
102-
std::visit(
103-
[&signal, &state_reference, id, uncommitted](auto&& wrapper) {
104-
auto callback = [&signal, &state_reference, id](const TypedResponse<TreeMetaResponse>& meta) {
105-
state_reference.insert({ id, { meta.inner.root, meta.inner.size } });
106-
signal.signal_decrement();
107-
};
108-
wrapper.tree->get_meta_data(uncommitted, callback);
109-
},
110-
tree);
106+
auto callback = [&signal, &state_reference, &state_ref_mutex, id](const TypedResponse<TreeMetaResponse>& meta) {
107+
std::lock_guard<std::mutex> lock(state_ref_mutex);
108+
state_reference.insert({ id, { meta.inner.root, meta.inner.size } });
109+
signal.signal_decrement();
110+
};
111+
std::visit([&callback, uncommitted](auto&& wrapper) { wrapper.tree->get_meta_data(uncommitted, callback); },
112+
tree);
111113
}
112114

113115
signal.wait_for_level(0);

yarn-project/world-state/scripts/build.sh

+4-3
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@ set -e
55
cd "$(dirname "$0")/.."
66

77
# relatiev path from the directory containing package.json
8-
WORLD_STATE_LIB_PATH=../../barretenberg/cpp/build/bin/world_state_napi.node
8+
WORLD_STATE_LIB_PATH=../../barretenberg/cpp/build-pic/lib/world_state_napi.node
9+
PRESET=${PRESET:-clang16-pic}
910

1011
build_addon() {
11-
(cd ../../barretenberg/cpp; cmake --preset clang16-pic -DCMAKE_BUILD_TYPE=RelWithAssert; cmake --build --preset clang16-pic --target world_state_napi; echo $PWD; mkdir -p build/bin; cp ./build-pic/lib/world_state_napi.node ./build/bin/world_state_napi.node)
12+
(cd ../../barretenberg/cpp; cmake --preset $PRESET -DCMAKE_BUILD_TYPE=RelWithAssert; cmake --build --preset $PRESET --target world_state_napi; echo $PWD; mkdir -p build/bin; cp ./build-pic/lib/world_state_napi.node ./build/bin/world_state_napi.node)
1213
}
1314

1415
cp_addon_lib() {
1516
if [ -f $WORLD_STATE_LIB_PATH ]; then
16-
echo "Copying world_state_napi.node to build directory"
17+
echo "Copying $(realpath $WORLD_STATE_LIB_PATH) to build directory"
1718
rm -rf build
1819
mkdir build
1920
cp $WORLD_STATE_LIB_PATH build/world_state_napi.node

0 commit comments

Comments
 (0)