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

feat: onSlotFinalized include all previous slots. #1335

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

panteleymonchuk
Copy link
Collaborator

Description of change

Mark all related blocks as finalized when we get slot index from onSlotFinalized handler.
Closes #1334

Type of change

  • Enhancement (a non-breaking change which adds functionality)

How the change has been tested

After handle onSlotFinalized function blocks for current slot Index and before index need to be marked as "finalized"

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

  • My code follows the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Member

@VmMad VmMad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works nice! Only suggestion is renaming some variables :)

Comment on lines +262 to +288
const slotsBefore = Array.from(confirmedBlocksBySlot.keys());

const slots = [...slotsBefore, slotFinalized];

const blocks = [];
for (const slot of slots) {
const blockIds = confirmedBlocksBySlot.get(slot);
if (blockIds) {
blocks.push(...blockIds);
}
}

if (blocks?.length) {
blocks.forEach((blockId) => {
const selectedColor = getBlockColorByState(themeMode, "finalized");
if (selectedColor) {
updateBlockIdToMetadata(blockId, {
state: "finalized",
color: selectedColor,
});
updateBlockColor(blockId, selectedColor);
}
});
}

for (const slot of slots) {
removeConfirmedBlocksSlot(slot);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks nice, I suggest renaming the variables a bit so it is more easy to understand :)

        const previousFinalizedSlots = Array.from(confirmedBlocksBySlot.keys());
        const allFinalizedSlots = [...previousFinalizedSlots, slotFinalized];
        const confirmedBlocks = [];

        for (const slot of allFinalizedSlots) {
            const confirmedBlockIds = confirmedBlocksBySlot.get(slot);

            if (confirmedBlockIds) {
                confirmedBlocks.push(...confirmedBlockIds);
            }
        }

        if (confirmedBlocks?.length) {
            confirmedBlocks.forEach((blockId) => {
                const finalizedBlockColor = getBlockColorByState(themeMode, "finalized");
                if (finalizedBlockColor) {
                    updateBlockIdToMetadata(blockId, {
                        state: "finalized",
                        color: finalizedBlockColor,
                    });
                    updateBlockColor(blockId, finalizedBlockColor);
                }
            });
        }

        for (const slot of allFinalizedSlots) {
            removeConfirmedBlocksSlot(slot);

@begonaalvarezd begonaalvarezd merged commit e3b34bf into dev Mar 26, 2024
4 of 6 checks passed
@begonaalvarezd begonaalvarezd deleted the feat/issues-1334-onSlotFinalized branch March 26, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: add onSlotFinalized hanlder [Task]: Update node colors to be based on block metadata
3 participants