feat: add air inspect admin ipc source#16
Open
kyonRay wants to merge 9 commits into
Open
Conversation
FIB-114: Missing Reentrancy Guard in Cache Operations Added a mutex to protect the global CacheExecutables cache during concurrent access. When multiple coroutines attempt to cache an executable for the same address simultaneously, the mutex ensures we don't have uncontrolled concurrent cache modifications. The fix uses a lock around the cache write operation to prevent race conditions. Concurrent cache misses may still result in duplicate code loading, but MemoryStorage with CONCURRENT attribute handles concurrent writes safely.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in local admin IPC transport for fisco-bcos-air inspect CLI (with fallback to rpc+logs), while also bundling multiple unrelated executor/txpool/gateway/security bugfixes and new hashing utilities.
Changes:
- Add
-cli inspectflow tofisco-bcos-air, including config loading, rendering, fallback inspectors, and an admin IPC client/server. - Introduce new storage2 primitives (
writeOneIf,insertIfAbsent) and apply them in nonce / cache handling. - Add rapidhash + update hashing and multiple regression tests across executor/txpool/gateway.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vcpkg.json | Add rapidhash dependency. |
| vcpkg-configuration.json | Register rapidhash overlay port. |
| ports/rapidhash/vcpkg.json | Define rapidhash port metadata. |
| ports/rapidhash/portfile.cmake | Fetch/install rapidhash header-only port. |
| fisco-bcos-air/CMakeLists.txt | Build/link new air-cli library into air node. |
| fisco-bcos-air/main.cpp | Route --cli into inspect CLI runner. |
| fisco-bcos-air/AirNodeInitializer.h | Add admin IPC server members/accessors. |
| fisco-bcos-air/AirNodeInitializer.cpp | Start/stop admin IPC server when enabled. |
| fisco-bcos-air/cli/InspectTypes.h | Define inspect response/sections model. |
| fisco-bcos-air/cli/InspectTypes.cpp | JSON serialization for inspect model. |
| fisco-bcos-air/cli/InspectRenderer.h | Declare renderers (human/JSON). |
| fisco-bcos-air/cli/InspectRenderer.cpp | Implement rendering logic. |
| fisco-bcos-air/cli/InspectConfig.h | Define inspect config + loader. |
| fisco-bcos-air/cli/InspectConfig.cpp | Parse ini + resolve relative paths. |
| fisco-bcos-air/cli/InspectApplication.h | CLI app orchestrator + source selection. |
| fisco-bcos-air/cli/InspectApplication.cpp | Implement fallback/admin-ipc execution paths. |
| fisco-bcos-air/cli/FallbackInspectors.h | Fallback inspectors API. |
| fisco-bcos-air/cli/FallbackInspectors.cpp | Implement degraded sections + log tailing. |
| fisco-bcos-air/cli/AdminInspectors.h | Admin-side inspectors API. |
| fisco-bcos-air/cli/AdminInspectors.cpp | Build in-process inspect response from node subsystems. |
| fisco-bcos-air/cli/AdminIPCProtocol.h | Define IPC request/reply schema + serialize hooks. |
| fisco-bcos-air/cli/AdminIPCProtocol.cpp | JSON (de)serialization for IPC protocol. |
| fisco-bcos-air/cli/AdminIPCClient.h | IPC client interface. |
| fisco-bcos-air/cli/AdminIPCClient.cpp | IPC client implementation via Boost.Asio local sockets. |
| fisco-bcos-air/cli/AdminIPCServer.h | IPC server interface + accept loop thread. |
| fisco-bcos-air/cli/AdminIPCServer.cpp | IPC server implementation on AF_UNIX sockets. |
| libinitializer/CommandHelper.h | Add CLIRequest + cliMode fields. |
| libinitializer/CommandHelper.cpp | Parse/normalize CLI args; add inspect CLI options. |
| tests/CMakeLists.txt | Link air-cli + command helper into test binary. |
| tests/unittest/CommandHelperTest.cpp | Unit tests for CLI parsing of inspect args. |
| tests/unittest/AirInspectCLITest.cpp | Unit tests for source selection + fallback sections. |
| bcos-framework/bcos-framework/storage2/Storage.h | Add writeOneIf and insertIfAbsent tags. |
| bcos-framework/bcos-framework/storage2/MemoryStorage.h | Implement tag_invoke for new storage2 operations. |
| bcos-framework/test/unittests/storage2/TestMemoryStorage.cpp | Add tests for writeOneIf. |
| bcos-utilities/bcos-utilities/BucketMap.h | Switch hashing to rapidhash + add lock upgrade helpers. |
| bcos-utilities/test/unittests/libutilities/BucketMapTest.cpp | Add test for seeded string hashing. |
| bcos-framework/bcos-framework/protocol/Protocol.h | Add block version 3.16.5 + bump defaults/max. |
| bcos-framework/bcos-framework/ledger/Features.h | Add new auth-related feature flags + upgrade roadmap. |
| bcos-framework/test/unittests/interfaces/FeaturesTest.cpp | Register new feature flag names in tests. |
| transaction-executor/bcos-transaction-executor/vm/VMInstance.cpp | Limit cached execution state by memory size. |
| transaction-executor/bcos-transaction-executor/vm/VMFactory.h | Catch evmone analyze exceptions; log + throw. |
| transaction-executor/bcos-transaction-executor/precompiled/PrecompiledImpl.h | Harden precompiled execution (gas validation, exception handling). |
| transaction-executor/bcos-transaction-executor/precompiled/AuthCheck.h | Include CREATE2 + reduce auth failure output leakage. |
| transaction-executor/bcos-transaction-executor/vm/HostContext.h | Pass features into auth checks + fix auth table path handling. |
| transaction-executor/bcos-transaction-executor/EVMCResult.h | Add error-output helper API; rename ctor param. |
| transaction-executor/bcos-transaction-executor/EVMCResult.cpp | Implement fillErrorOutputInPlace + adjust release printing. |
| transaction-executor/bcos-transaction-executor/TransactionExecutorImpl.h | Replace revert output fill logic to avoid leaks/double-free. |
| transaction-executor/tests/TestEVMCResult.cpp | Update expectation for release pointer behavior. |
| transaction-executor/tests/FIB77_81_AuthCheckTest.cpp | Add regression tests for CREATE2/auth revert status. |
| bcos-framework/bcos-framework/ledger/EVMAccount.h | Minor cast/copy fixes for raw address path building. |
| bcos-executor/src/precompiled/common/Utilities.h | Extend checkPathValid signature with features. |
| bcos-executor/src/precompiled/common/Utilities.cpp | Reject reserved _accessAuth suffix under feature flag. |
| bcos-executor/src/precompiled/BFSPrecompiled.cpp | Wire features into path validation. |
| bcos-executor/src/executive/TransactionExecutive.cpp | Auth table squatting mitigation + revert status handling. |
| bcos-executor/src/vm/Precompiled.h | Minor signature cleanups; registrar init guard braces. |
| bcos-executor/src/vm/Precompiled.cpp | Avoid operator[] inserting missing entries; use find(). |
| bcos-executor/test/unittest/libprecompiled/FIB83_AccessAuthSuffixTest.cpp | Add regression tests for _accessAuth path rejection. |
| bcos-gateway/bcos-gateway/libp2p/P2PMessage.cpp | Harden frame/options decoding bounds + version checks. |
| bcos-gateway/bcos-gateway/libp2p/P2PMessageV2.cpp | Propagate decodeHeader error from base class. |
| bcos-gateway/test/unittests/GatewayMessageTest.cpp | Adjust decode expectations + add malformed frame tests. |
| bcos-txpool/bcos-txpool/txpool/interfaces/NonceCheckerInterface.h | Make insert() return bool for atomic reserve. |
| bcos-txpool/bcos-txpool/txpool/validator/TxPoolNonceChecker.h | Update signature to bool insert. |
| bcos-txpool/bcos-txpool/txpool/validator/TxPoolNonceChecker.cpp | Implement atomic insert return semantics. |
| bcos-txpool/bcos-txpool/txpool/validator/TxValidator.cpp | Defer nonce insertion to post-validation stage. |
| bcos-txpool/bcos-txpool/txpool/validator/Web3NonceChecker.h | Change memory nonce keying + monotonic updates. |
| bcos-txpool/bcos-txpool/txpool/validator/Web3NonceChecker.cpp | Add nonce size guard; atomic insertIfAbsent; cache short-circuit. |
| bcos-txpool/bcos-txpool/txpool/storage/Web3Transactions.h | Make seal/remove synchronous (syncWait) and rename internal remove helper. |
| bcos-txpool/bcos-txpool/txpool/storage/MemoryStorage.cpp | Fix TOCTOU + pool limit ordering + sealing/unsealing concurrency guards. |
| bcos-txpool/test/unittests/txpool/Web3TransactionsTest.cpp | Update tests for non-coroutine seal/remove. |
| bcos-txpool/test/unittests/txpool/Web3NonceCheckerTest.cpp | Add concurrency + hashing + nonce normalization regression tests. |
| bcos-txpool/test/unittests/txpool/MemoryStorageTest.cpp | Add multiple regression tests around txpool fixes. |
| bcos-utilities/bcos-utilities/DataConvertUtility.h | Trailing newline/format cleanup. |
| .gitignore | Ignore Claude Code workspace artifacts. |
Comment on lines
+12
to
+29
| std::string readRequestLine(int fd) | ||
| { | ||
| std::string payload; | ||
| char ch = '\0'; | ||
| while (true) | ||
| { | ||
| auto bytesRead = ::read(fd, &ch, 1); | ||
| if (bytesRead <= 0) | ||
| { | ||
| break; | ||
| } | ||
| if (ch == '\n') | ||
| { | ||
| break; | ||
| } | ||
| payload.push_back(ch); | ||
| } | ||
| return payload; |
There was a problem hiding this comment.
readRequestLine() reads until \n with no maximum payload size, so a client can send an unbounded request line and force the server to allocate arbitrarily large memory (local DoS). Add a reasonable max request size (and fail the request if exceeded).
Comment on lines
+8
to
+55
| std::vector<std::string> readLogLines( | ||
| const std::string& logPath, std::optional<int> tail, const std::string& levelFilter) | ||
| { | ||
| namespace fs = std::filesystem; | ||
| std::vector<std::string> lines; | ||
| std::error_code ec; | ||
| fs::path path(logPath); | ||
| if (!fs::exists(path, ec)) | ||
| { | ||
| return lines; | ||
| } | ||
|
|
||
| std::vector<fs::path> files; | ||
| if (fs::is_regular_file(path, ec)) | ||
| { | ||
| files.push_back(path); | ||
| } | ||
| else if (fs::is_directory(path, ec)) | ||
| { | ||
| for (const auto& entry : fs::directory_iterator(path, ec)) | ||
| { | ||
| if (entry.is_regular_file(ec)) | ||
| { | ||
| files.push_back(entry.path()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::sort(files.begin(), files.end()); | ||
| for (const auto& file : files) | ||
| { | ||
| std::ifstream in(file); | ||
| std::string line; | ||
| while (std::getline(in, line)) | ||
| { | ||
| if (!levelFilter.empty() && line.find(levelFilter) == std::string::npos) | ||
| { | ||
| continue; | ||
| } | ||
| lines.push_back(line); | ||
| } | ||
| } | ||
|
|
||
| if (tail && *tail > 0 && static_cast<size_t>(*tail) < lines.size()) | ||
| { | ||
| lines.erase(lines.begin(), lines.end() - *tail); | ||
| } | ||
| return lines; |
There was a problem hiding this comment.
readLogLines() loads all lines from every file under logPath into memory and only then applies tail, which can be very slow and memory-heavy for large log directories. Consider applying tail while streaming (e.g., keep a fixed-size ring buffer/deque of the last N matching lines, or seek from file end when tail is set).
| @@ -0,0 +1,138 @@ | |||
| #include "FallbackInspectors.h" | |||
There was a problem hiding this comment.
readLogLines() uses std::sort but this file does not include <algorithm>, which can break builds depending on transitive includes. Add the missing standard header (or otherwise avoid relying on indirect includes).
Suggested change
| #include "FallbackInspectors.h" | |
| #include "FallbackInspectors.h" | |
| #include <algorithm> |
kyonRay
force-pushed
the
release-3.17.0
branch
2 times, most recently
from
a8354c2 to
0e8d700
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
./fisco-bcos -cli inspectrpc+logswhen admin IPC is disabled or unreachable, and resolve relative inspect paths from the config directoryVerification
cmake --build build-make --target fisco-bcos-test fisco-bcos -j4./build-make/tests/fisco-bcos-test --run_test=selectsRpcAndLogsWhenAdminDisabled,logInspectorReadsConfiguredPath,selectsAdminIpcWhenEnabledAndReachable,fallsBackWhenAdminIpcUnavailable,fallbackResponseReportsRpcAndLogsSourceEvenWhenAdminEnabled --log_level=test_suite./build-make/fisco-bcos-air/fisco-bcos -c <tmp>/config.ini -g tools/BcosBuilder/src/tpl/config.genesis -cli inspect logs --json --tail 2 --level INFOagainst a temp local log dir and confirmsource=rpc+logswith filtered entries