fix: finalization for /blocks/head
#631
Conversation
| const { hash, queryFinalizedHead, omitFinalizedTag } = | ||
| await this.parseFinalizationOpts(this.options.finalizes, paramFinalized); | ||
| await this.parseFinalizationOpts( |
There was a problem hiding this comment.
I don't get this, how does it fix it by converting passing it as a String rather than a boolean?
There was a problem hiding this comment.
hmmm, you make a good point that it is confusing too outside eyes as to what is happening, so I am going to just revert it completely to what it use to be so that there is no confusion.
There was a problem hiding this comment.
I'm a bit confused about the string usage; can the string just be true or false and, why a string and not a bool? I'm probably missing too much context really :)
Sounds like you'll revert anyway so maybe I don't need to know :D
There was a problem hiding this comment.
@niklasad1 @jsdw Okay, so I reverted it back to what it was originally before I abstracted out of the controller. Hopefully it's a little clearer
There was a problem hiding this comment.
So IIUC, finalizes checks if the network itself supports finalization, and finalized is a user parameter, but I'm not sure what the use of FinalizedTag/queryFinalizedHead is. What does FinalizedTagdo that queryFinalizedHeaddoes not? I'm trying to understand why we use queryFinalizedHead with the chain.getHeader() func. Or why when we use getHeaderwhen queryFinalizedHead head is false, and getFinalizedHead when queryFinalizedHead is true.
Otherwise I think this looks good
|
@insipx Great question, I just added a comment to clarify. But the idea behind The logic where this all takes place when this is queried ( |
This fixes the default behavior for querying the finalized head. The query param and its expected behavior still works. The only issue is the
/blocks/headendpoint is not returning the finalized head by default, which means you have to tag the finalized param onto the query. This fixes the slight bug introduced in this PR #615.