`Packet.timeoutHeight` should have type `Option<Height>` (or equivalent)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1k
- Forks
- 457
- Avg merge
- 4d 5h
- Merged PRs (30d)
- 29
Description
Packet.timeoutHeight treats the minimum height 0 specially: a height of 0 means "no timeout". This is taken from this check in timeoutPacket():
abortTransactionUnless(
(packet.timeoutHeight > 0 && proofHeight >= packet.timeoutHeight) ||
(packet.timeoutTimestamp > 0 && connection.getTimestampAtHeight(proofHeight) > packet.timeoutTimestamp))
This is inconsistent with the ICS-2 definition of Height, which specifies that 0 is a valid height. It would be cleaner to represent this special case directly in the type: anything equivalent to Rust's Option<Height> (or Option<NonZeroHeight> depending on if we want to allow a timeout at height 0 or not).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the Packet.timeoutHeight definition and timeoutPacket(), then review the ICS-2 Height semantics referenced in the issue. Determine whether the intended representation is Option or Option, update the affected callers and tests, and verify that height 0 and timeout heights retain the intended behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- blockchain
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100