Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate the pipeline events into block and transaction events #4315

Closed
DCNick3 opened this issue Feb 21, 2024 · 0 comments
Closed

Separate the pipeline events into block and transaction events #4315

DCNick3 opened this issue Feb 21, 2024 · 0 comments
Assignees
Labels
api-changes Changes in the API for client libraries iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix Refactor Improvement to overall code quality

Comments

@DCNick3
Copy link
Contributor

DCNick3 commented Feb 21, 2024

Also consider switching the block events filtering to use height, as there's no real way to predict block hashes.


    // IMO it should have the suffix `Box`
    pub enum PipelineEventBox {
        Block(BlockEvent),
        Transaction(TransactionEvent),
    }

    struct BlockEvent {
        pub status: BlockStatus,
        // NOTE: Does it make sense to have hash filtering for blocks?
        // because it's unpredictable which block will be constructed
        // It might be only relevant when replaying blocks in which case we can use height
        // pub hash: HashOf<SignedBlock>,
        pub height: u64,
    }

    struct TransactionEvent {
        pub status: TransactionStatus,
        pub hash: HashOf<SignedTransaction>,
    }

    // FIXME: Some variants are never actually emitted. Fix this in the codebase
    enum BlockStatus {
        Validating,
        Committed,
        Rejected(crate::block::error::BlockRejectionReason),
    }

    // FIXME: Some variants are never actually emitted. Fix this in the codebase
    enum TransactionStatus {
        Validating,
         // it's factually incorrect to say that transaction was committed unless block get committed
        Approved,
        // Rejected transactions are also committed
        Rejected(crate::transaction::error::TransactionRejectionReason),
        // I think we can emit a batch of committed transaction events at the time of block commit
        Committed,
    }

I think this makes more sense

Originally posted by @mversic in #4240 (comment)

@DCNick3 DCNick3 added iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries Refactor Improvement to overall code quality labels Feb 21, 2024
@mversic mversic assigned mversic and unassigned mversic Feb 27, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 14, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 19, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 21, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 22, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 25, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 26, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 26, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 27, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 27, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 27, 2024
mversic added a commit to mversic/iroha that referenced this issue Mar 27, 2024
mversic added a commit that referenced this issue Mar 28, 2024
@timofeevmd timofeevmd self-assigned this Apr 1, 2024
@timofeevmd timofeevmd added the QA-confirmed This bug is reproduced and needs a fix label Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix Refactor Improvement to overall code quality
Projects
None yet
Development

No branches or pull requests

3 participants