bitshares / bitshares/bitshares-core

ElasticSearch plugin startup check is incomplete

Open
#2,494 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
1.2k
Forks
660
Avg merge
8h 17m
Merged PRs (30d)
26

Description

**Bug Description**

The `checkES()` function returns `true` even if there is an error in the response: https://github.com/bitshares/bitshares-core/blob/c06d09cecdd9e7526a23456986eb2f9b2b6894aa/libraries/utilities/elasticsearch.cpp#L39-L49

https://github.com/bitshares/bitshares-core/blob/c06d09cecdd9e7526a23456986eb2f9b2b6894aa/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp#L535-L543

It is inappropriate to use the `_nodes` query to check, because the user may not have specific permissions.

Actually the check should be moved to `plugin_initialize()`, because `plugin_startup()` is called after `database::open()` which could do a partial replay which would already need to insert data into ES. Update: this has been addressed via https://github.com/bitshares/bitshares-core/pull/1997/commits/679e6459adf38641448cae0283d812296236c80e in #1997.

Note: the same check is also used in `elasticsearch_plugin::get_account_history(...)`: https://github.com/bitshares/bitshares-core/blob/1d1fb99900f4aaa28fc3ea18169537994aaf6817/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp#L620-L623

By the way, the same check in `es_objects` has a line `es.auth = my->_es_objects_index_prefix;` which is obviously wrong. This has been fixed via https://github.com/bitshares/bitshares-core/pull/2570/commits/8cc9e4cac02de11cf2b631e2d39032a6e7053430 in #2570.
https://github.com/bitshares/bitshares-core/blob/6b617724418d9ccbe1bb84f10ae965a0e9ceabfd/libraries/plugins/es_objects/es_objects.cpp#L403-L414

**Impacts**
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.
- [ ] API (the application programming interface)
- [ ] Build (the build process or something prior to compiled code)
- [ ] CLI (the command line wallet)
- [ ] Deployment (the deployment process after building such as Docker, Travis, etc.)
- [ ] DEX (the Decentralized EXchange, market engine, etc.)
- [ ] P2P (the peer-to-peer network for transaction/block propagation)
- [ ] Performance (system or user efficiency, etc.)
- [ ] Protocol (the blockchain logic, consensus, validation, etc.)
- [ ] Security (the security of system or user data, etc.)
- [ ] UX (the User Experience)
- [ ] Other (please add below)

## CORE TEAM TASK LIST
- [ ] Evaluate / Prioritize Bug Report
- [ ] Refine User Stories / Requirements
- [ ] Define Test Cases
- [ ] Design / Develop Solution
- [ ] Perform QA/Testing
- [ ] Update Documentation

Contributor guide

No contributing guide indexed for this repository

Research direction

Read checkES() in libraries/utilities/elasticsearch.cpp and its uses in libraries/plugins/elasticsearch/elasticsearch_plugin.cpp, including plugin startup and get_account_history(...). Compare the reported behavior with the updates referenced in pull requests #1997 and #2570. Done means the startup and account-history checks correctly handle response errors without requiring the _nodes query or incorrect es_objects authentication data.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, elasticsearch
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.