fix: limit vsc matured packets handled per endblocker

[shaspitz]

Description

Closes: #1001

Note this PR will need to be backported to the v2 release branch

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• Included the correct type prefix in the PR title
• Added ! to the type prefix if API or client breaking change
• Confirmed this PR does not introduce changes requiring state migrations, OR migration code has been added to consumer and/or provider modules
• Targeted the correct branch (see PR Targeting)
• Provided a link to the relevant issue or specification
• Followed the guidelines for building SDK modules
• Included the necessary unit and integration tests
• Added a changelog entry to CHANGELOG.md
• Included comments for documenting Go code
• Updated the relevant documentation or specification
• Reviewed "Files changed" and left comments if necessary
• Confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed this PR does not introduce changes requiring state migrations, OR confirmed migration code has been added to consumer and/or provider modules
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage

[shaspitz]

Not sure if this is strictly neccessary, but it will ensure that we change throttling behavior as little as possible

[mpoke]

What's the reasoning behind this check? If during HandleLeadingVSCMaturedPackets there were 100 packets handled, then without this line we'll handle one more in this block, right?

[shaspitz]

The behavior you've described sounds correct as far as what would happen. I added this check here because it'll keep the handling order as similar as possible to what existed previously

[shaspitz]

ie. before this PR we always handle all VSC matured packets that're at the head of the queue, before any slash packets. This line keeps that behavior consistent. Is it strictly needed? Probably and hopefully not, just good practice

[shaspitz]

I'm not a fan of assuming that seqNums will be constructed s.t. the value at index 0 always corresponds to the slash packet, and all trailing indexes correspond to vsc matured.

The tests around GetSlashAndTrailingData do validate the behavior I've described. However a better solution would be to refactor GetSlashAndTrailingData so that it returns structs that look something like..

type slashPacketDataWithSeqNum struct {
	ibcSeqNum uint64
	data      ccvtypes.SlashPacketData
}

type vscMaturedPacketDataWithSeqNum struct {
	ibcSeqNum uint64
	data      ccvtypes.VSCMaturedPacketData
}

This is a trivial code change, but it'll likely make a large diff in throttle_test.go

[mpoke]

Nice work @smarshall-spitzbart. The logic seems good. I think we don't need to store VSCMaturedHandledThisBlock.

[MSalopek]

After a pair review session I agree with these changes.

The overall throttling logic can be refactored, but that's for another PR.

Approval

[jtremback]

LGTM