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

Rename our SlotNo to SlotInEpoch #1887

Merged
merged 1 commit into from
Jul 10, 2020
Merged

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jul 8, 2020

Issue Number

#1868

Overview

  • Renamed SlotNo to SlotInEpoch (for consistency with other components)

Comments

  • W.SlotNo == Cardano.SlotInEpoch /= Cardano.SlotNo
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.

Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to match the names in ouroboros-consensus.

It would be nice if there were a small library of slotting types that we could import.

@erikd
Copy link
Contributor

erikd commented Jul 9, 2020

It would be nice if there were a small library of slotting types that we could import.

https://github.com/input-output-hk/cardano-base/tree/master/slotting maybe?

@Anviking Anviking force-pushed the anviking/1868/rename-slotNo branch from 9a19a63 to 48d3ccd Compare July 9, 2020 08:57
@Anviking
Copy link
Member Author

Anviking commented Jul 9, 2020

https://github.com/input-output-hk/cardano-base/tree/master/slotting maybe?

Let's not get ahead of ourselves 😄 but I'm hoping we can switch to that soon.

Think we might end up with a few orphans in the wallet (e.g. for a Buildable instance), or the need to resolve that somehow.

@Anviking
Copy link
Member Author

Anviking commented Jul 9, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 9, 2020
1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


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

iohk-bors bot commented Jul 9, 2020

Build failed

@Anviking
Copy link
Member Author

Anviking commented Jul 9, 2020

bors r+

@Anviking Anviking added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Jul 9, 2020
iohk-bors bot added a commit that referenced this pull request Jul 9, 2020
1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


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

iohk-bors bot commented Jul 9, 2020

Build failed

@Anviking Anviking force-pushed the anviking/1868/rename-slotNo branch from 48d3ccd to 8a85564 Compare July 9, 2020 14:14
@Anviking
Copy link
Member Author

Anviking commented Jul 9, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 9, 2020
1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


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

iohk-bors bot commented Jul 9, 2020

Build failed

@Anviking
Copy link
Member Author

bors r+
(master has changed, should not be cached anymore I think)

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
1847: Track Stakepool Retirements in the DB r=jonathanknowles a=jonathanknowles

# Related Issues

#1815 
#1816 
#1817 
#1819 
#1880 

# Overview

This PR:

- [x] Extends the pool DB schema to make it possible to store:
    - [x] retirement certificates.
    - [x] the _order_ of publication of certificates _within the same block_, for both registration and retirement certificates. This is necessary, as the intra-block order is _significant_.<br><br>

- [x] Adds the `PoolLifeCycleStatus` type, which indicates the current lifecycle stage of a pool.

    There are currently _three_ possibilities:
    1. `NotRegistered`
        (indicates that a pool has never been registered)
    2. `Registered`
        (indicates that a pool has been registered but _not_ retired)
        (provides a registration certificate)
    3. `RegisteredAndRetired`
        (indicates that a pool has been registered _and_ retired)
        (provides both a registration and a retirement certificate)<br><br>

- [x] Adds the `readPoolLifeCycleStatus` function to the pool DB layer, making it possible to determine the current lifecycle status of a pool. 

- [x] Updates the `retirement` field of `ApiStakePool` with live information.

# Property Tests

This PR adds property tests to ensure that:

- [x] the `determinePoolLifeCycleStatus` function respects the correct order of precedence for registration and retirement certificates:
    - for a given pool, a registration certificate always _supercedes_ a prior retirement certificate.
    - for a given pool, a retirement certificate always _augments_ the latest registration certificate.
- [x] `readPoolLifeCycleStatus` queries work correctly in the presence of blocks containing multiple certificates for the same pool.
- [x] database rollback works correctly in the presence of retirement certificates.

# Future Work

A **_future_** PR will add integration tests to ensure that:

- retiring a pool eventually results in a correct update to the `retirement` field of `ApiStakePool`, when read through the `ListStakePools` operation.
- retiring and then re-registering a pool clears the `retirement` field. 

See QA section of #1819.

1849: Diverse witnesses in shelley r=paweljakubas a=paweljakubas

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->
#1844 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have added WitnessTag
- [x] I have added impl of mkTx for Byron/Icarus wtinesses
- [x] I have added unit tests verifying decodeTx for Byron/Icarus witnesses
- [x] I have updated fee estimation
- [x] I have added integration tests illustrating the case 


# Comments

<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Jonathan Knowles <[email protected]>
Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: IOHK <[email protected]>
Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

Build failed (retrying...)

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
1849: Diverse witnesses in shelley r=paweljakubas a=paweljakubas

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->
#1844 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have added WitnessTag
- [x] I have added impl of mkTx for Byron/Icarus wtinesses
- [x] I have added unit tests verifying decodeTx for Byron/Icarus witnesses
- [x] I have updated fee estimation
- [x] I have added integration tests illustrating the case 


# Comments

<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Pawel Jakubas <[email protected]>
Co-authored-by: IOHK <[email protected]>
Co-authored-by: KtorZ <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

Build failed (retrying...)

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
1887: Rename our SlotNo to SlotInEpoch r=Anviking a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


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

iohk-bors bot commented Jul 10, 2020

Build failed

The SlotNo we use is different from the one in consensus. By renaming it
to SlotInEpoch it will be less confusing, and easier for us to start
adoping the actual SlotNo.

With an epoch length of 2, this is how they differ:

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.
@KtorZ KtorZ force-pushed the anviking/1868/rename-slotNo branch from 8a85564 to 357bb22 Compare July 10, 2020 17:28
@KtorZ
Copy link
Member

KtorZ commented Jul 10, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
1887: Rename our SlotNo to SlotInEpoch r=KtorZ a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <[email protected]>
@KtorZ
Copy link
Member

KtorZ commented Jul 10, 2020

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

Canceled

@KtorZ
Copy link
Member

KtorZ commented Jul 10, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 10, 2020
1826: Add more debug-level chain-sync- and follow traces r=KtorZ a=Anviking

# Issue Number

Related to / was used to debug #1794 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

The chain-following code is complex. We usually log when the `follow` function asks us to roll forward or backward, but we don't have traces for the underlying logic.

- [x] Trace when `follow` ignores (hopefully) idempotent rollbacks, and why
- [x] Trace low-level chain-sync commands


# Comments

<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


1887: Rename our SlotNo to SlotInEpoch r=KtorZ a=Anviking


# Issue Number

#1868
<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Renamed `SlotNo` to `SlotInEpoch` (for consistency with other components)


# Comments

- `W.SlotNo` == `Cardano.SlotInEpoch` /= `Cardano.SlotNo`

```
Epoch       0   1   2   3   4
SlotNo*     0 1 2 3 4 5 6 7 8 9
SlotInEpoch 0 1 0 1 0 1 0 1 0 1
```

*) I.e. the one defined in cardano-base and used in ourobouros-consensus.


<!-- 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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


1895: hydra: Prevent caching of shelley integration test failures r=KtorZ a=rvl

### Issue Number

Not sure.

### Overview

This change causes integration tests to be re-run whenever the git revision changes, even if everything else is identical. Since these tests tend to fail a lot, we don't want to cache false failures.

Also increase the minimum severity for integration test logging, because debug level produces quite a lot of output.


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

iohk-bors bot commented Jul 10, 2020

Build failed (retrying...)

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 10, 2020

@iohk-bors iohk-bors bot merged commit 62f9320 into master Jul 10, 2020
@iohk-bors iohk-bors bot deleted the anviking/1868/rename-slotNo branch July 10, 2020 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants