Improved Filtering and Selection Logic for Best Tip and Tie Breaker Mechanism #80
Conversation
This file contains 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
MartinMinkov
force-pushed
the
feat/selection-algo
branch
from
db3a664
to
b833fa7
Compare
MartinMinkov
force-pushed
the
feat/selection-algo
branch
from
b833fa7
to
d35b10c
Compare
bkase
requested changes
Apr 25, 2023
There was a problem hiding this comment.
This change looks good to me but we should definitely ask @nholland94 to confirm.
However, before we land this we need to do something to mitigate inevitable incompatibilities that will creep in as the consensus rules change.
Given that we don’t want to introduce a wasm nor jsofocaml runtime dependencies to this server, what I think we need is some kind of diff test between another well-tested implementation and this one. The choices are: the real implementation in Mina OR the selection algorithm in openmina (after verifying that that implementation is also bound to the main implementation in some way). This is one of those times I wish we had some kind of executable spec that we could derive multiple implementations from.
|
Sounds like we should start planning for a framework to testing multiple implementations? |
4 tasks
4 tasks
…ode-adapter and import it from mina-consensus
…nsus rules refactor(archive-node-adapter/index.ts): move helper functions to mina-consensus.ts for better organization refactor(archive-node-adapter/index.ts): split getEvents into getEventData and sortAndFilterEvents for better readability refactor(archive-node-adapter/index.ts): add getTraceInfo method to handle traceInfo extraction from options
…mpatibility feat(archive-node-adapter): add null checks and initialization for services to ensure they are ready before use refactor(archive-node-adapter): change tracingService, eventsService, actionsService to nullable for better error handling feat(resolvers): add traceInfo to getActions call for better tracing refactor(tracing): rename spans to spanStack for better semantics test: add console log to events test for better debugging
…cript support in the project
…or better performance and smaller image size refactor(Dockerfile): copy only necessary files (src and tsconfig.json) to build stage for cleaner build fix(Dockerfile): copy node_modules from build stage to avoid unnecessary production dependencies installation feat(Dockerfile): add non-root user for better security style(Dockerfile): add newline at end of file to follow best practices
This commit introduces a new docker-compose.dev.yml file that defines the services required for the development environment. This includes services for archive, postgres, jaeger, and the main app. Each service is configured with necessary environment variables, ports, volumes, and networks. This setup allows for an isolated and consistent development environment.
…omments refactor(mina-consensus.ts): modify compareBlockChainLength function to compare height instead of distanceFromMaxBlockHeight for better accuracy refactor(events-service.ts): remove unused import 'util' to clean up code
…ed.ts for consensus algorithm 1. precomputed-block.ts: Define types and interfaces for precomputed block data to improve type safety and readability. 2. run-consensus-precomputed.ts: Implement a script to read block data, validate blocks, convert them into OCaml block type, initialize PoS data structures, run PoS selection algorithm, and print out the results. This is to automate the process of running consensus algorithm on precomputed blocks.
…to enable TypeScript compilation for consensus tests
…form way fix(config.ts): change the path of the config file to 'src/consensus/config.mlh' to correctly locate the configuration file
…le multiple events/actions The function now correctly handles multiple events/actions emitted in a single account update. It also ensures that the events/actions are placed back in the correct order within the transaction. This change was necessary to accurately represent the sequence of events/actions within a transaction.
MartinMinkov
force-pushed
the
feat/selection-algo
branch
from
b44a1b6
to
05fa04b
Compare
…-adapter/utils' to 'services/utils/utils' for better project structure refactor(utils.test.ts): change import path for Action, Event from 'models/types' to 'blockchain/types' to reflect new file organization
…BlockStatusFilter due to directory structure changes for better organization and readability
…fresh builds refactor: replace absolute imports with relative imports for better compatibility across different environments style: remove unnecessary blank lines in resolvers.test.ts test(utils.test.ts): add type casting to ArchiveNodeDatabaseRow[] for test data style(tsconfig.json): remove trailing whitespace
… two values with a condition This function is added to provide a way to compare two values using a provided compare function and a condition. This will be useful in scenarios where we need to compare two values and decide which one to return based on a condition.
…queries.ts, db/sql/events-actions/types.ts): add lockCheckpoint field to BlockInfo and ArchiveNodeDatabaseRow types, and update SQL queries to include lock_checkpoint This change is done to include the lock_checkpoint data in the block information and archive node database row, which is necessary for the new feature of lock checkpointing in the blockchain.
… epoch transition validation refactor(consensus): update select function to handle short range fork and epoch transition fix(db): update SQL queries to include next_epoch_data_id for accurate block data retrieval test(consensus): update test scripts to reflect changes in block data and consensus selection
… to reflect changes in tie breaker rules The comments were updated to reflect the new tie breaker rules for chain selection in the Mina Protocol. The new rules consider whether the chains fork at a short range or a long range, and apply different tie breaker rules accordingly. The old comments, which only considered the highest VRF output and state hash, were removed.
…ns from utils.ts to database-row-adapters.ts for better organization feat(database-row-adapters.ts): add new file to handle database row operations, improving code modularity and readability fix(select-consensus-precomputed.ts): remove unused properties from mapPrecomputedToBlockInfo function to improve performance and reduce complexity
…nd initialize spanStack in declaration feat(tracing-service.ts): add return type void to startSpan and endSpan methods for better type safety fix(tracing-service.ts): throw new Error instead of Error in endSpan method for better error handling feat(tracing-service.ts): add error handling for case when span is not retrieved from the stack in endSpan method
…etter code organization refactor(server.ts): make buildServer and buildPlugins async to support async tracing setup refactor(tracing): delete old tracing index file and replace with new jaeger-tracing module feat(tracing): add Jaeger setup functions to validate config, create exporter, parse endpoint, and check endpoint availability refactor(tracing): move tracing related functions and types to jaeger-tracing module refactor(archive-node-adapter.ts, resolvers.ts, tracing-service.ts): update import paths to reflect new jaeger-tracing module location
…tial errors fix(index.ts): change process.exit code from 1 to 0 on normal termination to follow standard conventions feat(index.ts): add error logging in catch block to improve error visibility and debugging
…ve clarity The comments for the select function have been updated to provide a more concise and clear explanation of the function's purpose and behavior. The new comments also include a step-by-step breakdown of the chain comparison process.
…onger needed in the project
…for improved performance and security refactor(context.ts, jaeger-tracing.ts): change import from '@opentelemetry/tracing' to '@opentelemetry/sdk-trace-base' due to package update refactor(index.ts): move server building logic to separate file for better code organization feat(server/plugins.ts, server/server.ts): add new files to handle server plugins and server creation separately for better code modularity refactor(jaeger-setup.ts): simplify response handling in checkJaegerEndpointAvailability function for cleaner code
…' to 'graphql' for better consistency with other services
feat(resolvers.ts): add try-finally block to ensure parentSpan ends even if an error occurs fix(jaeger-tracing.ts): move setGlobalTracerProvider after provider registration for correct order of operations
…s to reduce project dependencies and improve load time
…reflect their new location in 'database-row-adapters' for better organization and clarity
…n and export buildContext and GraphQLContext directly refactor(archive-node-adapter.ts): remove tracingService and initializeServices method, simplify getEvents and getActions methods refactor(resolvers.ts): replace getCurrentSpanFromGraphQLContext with setSpanNameFromGraphQLContext, refactor events and actions methods refactor(actions-service.ts): remove tracingService dependency, refactor getActions and getActionData methods, remove return statement from sortAndFilterBlocks function refactor(events-service.ts): remove tracingService dependency, refactor getEvents and getEventData methods refactor(jaeger-setup.ts): remove createJaegerExporter function refactor(jaeger-tracing.ts): remove createTraceInfo, getGlobalTracer, getTraceInfoFromOptions functions and TraceInfo type feat(tracer.ts): add new file to handle tracing logic, introduce TracingState class, extractTraceStateFromOptions and setSpanNameFromGraphQLContext functions The changes were done to simplify the codebase, remove unnecessary dependencies and improve the tracing logic. The new TracingState class encapsulates the tracing logic, making it easier to manage and understand. The refactoring of the services and resolvers improves the readability and maintainability of the code.
…ny type for plugins array refactor(server.ts): import Plugin from '@envelop/core' instead of 'graphql-yoga' for better type compatibility refactor(server.ts): remove object type from Plugin in buildServer function to allow any type of Plugin
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.
Description
In this PR, we have enhanced the filtering and selection logic for the best tip and refactored the code for better readability and maintainability. The primary focus is on handling multiple blocks at the highest tip of the network (the latest block height) and implementing a tiebreaker mechanism to decide the optimal block to proceed with. This is crucial in the context of events/actions, as users rely on the data emitted in the latest block available, requiring an accurate representation of the network.
The tie-breaking process for blocks with duplicate heights at the maximum level is discussed in detail here:
Consensus Tie Breaker Rules
As outlined, the
lastVRFproperty for each block is examined, and the highest Blake2B hash value is selected. In the unlikely event of a tie, the higher block hash is chosen. The select function in the Mina daemon illustrates these rules:Select based on VRF Output
Select based on Block Hash
To facilitate this process, we introduced the blakejs dependency, which is used to construct a Blake2B hash and convert it into hex format for each lastVRFOutput. We then analyze all the latest blocks and run the comparison function.