fix(store): correctly break weight ties based on smaller ticket

[arajasek]

Related Issues

@rjan90 and @jennijuju reported seeing this log a lot, which prompted an investigation of the logic. This code does not implement FIP-0023 correctly. Specifically, if we have the following mintickets:

ts1: 1, 2
ts2: 0,3

This code will incorrectly return ts1 as the "smaller" tipset, even though the tie should have been broken in favour of ts2 based on 1 > 0.

Proposed Changes

Fix this bug, we should return in favour of ts2 if its ticket is smaller. Add test.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

• Commits have a clear commit message.
• PR title is in the form of of <PR type>: <area>: <change being made>
  • example: fix: mempool: Introduce a cache for valid signatures
  • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
  • area, e.g. api, chain, state, mempool, multisig, networking, paych, proving, sealing, wallet, deps
• Update CHANGELOG.md or signal that this change does not need it.
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • If the change does not require a CHANGELOG.md entry, do one of the following:
    • Add [skip changelog] to the PR title
    • Add the label skip/changelog to the PR
• New features have usage guidelines and / or documentation updates in
  • Lotus Documentation
  • Discussion Tutorials
• Tests exist for new functionality or change in behavior
• CI is green

[rvagg]

matches FIP-0023 from my read of it

needs a fiximports though on the new test file

[Stebalien]

@ribasushi pointed out that this is likely consensus breaking and I think he's right. Won't this cause us to order blocks within a tipset differently?

[Stebalien]

Ah! This is between two tipsets. It's a "soft" consensus breaking change that'll likely resolve within a few tipsets, from what I can tell.

[Stebalien]

(more importantly, the current behavior is forky and the new behavior is less forky)

[arajasek]

Ah! This is between two tipsets. It's a "soft" consensus breaking change that'll likely resolve within a few tipsets, from what I can tell.

Yeah, the other thing is that this is already the fallback option. Weight is what's actually supposed to decide between tipsets, this is only for when that ties.