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

Increase artificialEpochLength to maxBound :: Word32 #1046

Merged
merged 2 commits into from
Nov 19, 2019

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Nov 18, 2019

Issue Number

#1025

Bug to be created. In the mean-time https://input-output-rnd.slack.com/archives/GBT05825V/p1574113682014100

Overview

  • I have increased artificialEpochLength to maxBound :: Word32

Comments

  • I thought the explicit :: Word16 annotation was intentional from the beginning, but its usefulness could now be debated.
  • ⚠️ Regression test to be added. Why wasn't this caught by existing tests somehow?

@Anviking Anviking self-assigned this Nov 18, 2019
@rvl
Copy link
Contributor

rvl commented Nov 19, 2019

I removed the pool sqlite types module altogether, because it is a subset of the wallet sqlite types module (except for the bug which was fixed in one but not the other).

The state machine model tests could have picked up the error, were the SlotId generator not limiting slot numbers to the range 0-10.

@jonathanknowles
Copy link
Member

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 19, 2019
1046: Increase artificialEpochLength to `maxBound :: Word32` r=jonathanknowles a=Anviking

# Issue Number

Bug to be created. In the mean-time https://input-output-rnd.slack.com/archives/GBT05825V/p1574113682014100

# Overview

- [x] I have increased `artificialEpochLength` to `maxBound :: Word32`

# Comments

- I thought the explicit `:: Word16` annotation was intentional from the beginning, but its usefulness could now be debated.
- ⚠️ Regression test to be added. Why wasn't this caught by existing tests somehow?

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 19, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit 98c89bb into master Nov 19, 2019
@jonathanknowles jonathanknowles deleted the anviking/fix-artificial-epoch-length branch November 19, 2019 04:37
@KtorZ KtorZ added this to the Support Delegated Addresses milestone Dec 9, 2019
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.

4 participants