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

Join stake pool shelley #1722

Merged
merged 22 commits into from
Jun 12, 2020
Merged

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Jun 3, 2020

Issue Number

#1709

Overview

  • I have mocked stake pool layer and added joinStakePool to shelley api server
  • I have added first tests (now wallet id and no pool id) for stake pools in shelley
  • I have added impl for mkDelegationJoinTx
  • I have extended mkDelegationJoinTx by adding WalletDelegation
  • I have added tests for joining/rejoining
  • I have impl quitting and added tests

Comments

@paweljakubas paweljakubas added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jun 3, 2020
@paweljakubas paweljakubas self-assigned this Jun 3, 2020
@paweljakubas paweljakubas changed the title Paweljakubas/1709/join stake pool shelley Join stake pool shelley Jun 3, 2020
@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch 2 times, most recently from 66ef978 to bba630e Compare June 3, 2020 18:03
@paweljakubas
Copy link
Contributor Author

paweljakubas commented Jun 3, 2020

I need to still improve on mkDelegationJoinTx as I have :

        from the following response: Left (DecodeFailure "{\"code\":\"created_invalid_transaction\",\"message\":\"That's embarrassing. It looks like I've created an invalid transaction that could not be parsed by the node. Here's an error message that may help with debugging: ApplyTxError [LedgerFailure (DelegsFailure (DelegateeNotRegisteredDELEG (KeyHash 89eb0d6a8a691dae2cd15ed0369931ce0a949ecafa5c3f93f8121833646e15c3))),LedgerFailure (UtxowFailure (UtxoFailure (FeeTooSmallUTxO (Coin 143200) (Coin 122500))))]\"}")

@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from bba630e to 5791948 Compare June 3, 2020 19:27
(k, pwd) <- lookupPrivateKey keyFrom addr
pure $ mkWitness unsigned (getRawKey k, pwd)

witsForChimeric <- fmap Set.fromList (pure $ [mkWitness unsigned (getRawKey accXPrv, pwd')])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is probably the culprit for too low fee - I am adding this artificially

@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch 2 times, most recently from eaf01ae to 8c8d70e Compare June 8, 2020 12:07
-> [TxOut]
-> Either ErrMkTx (Tx, SealedTx)
_mkDelegationJoinTx poolId (accXPrv, pwd') keyFrom slot ownedIns outs = do
let timeToLive = (toSlotNo epochLength slot) + 7200
Copy link
Member

Choose a reason for hiding this comment

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

I know this is still a draft, but I hope the similarities between mkDelegationJoinTx and mkStdTx can be factored out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, some code overlap - I will reuse it later

VerKeyEd25519DSIGN .
throwCryptoError $
Ed25519.publicKey pId
--}
Copy link
Member

Choose a reason for hiding this comment

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

I'd definitely factor these functions out to make the body of this function more readable 👍

:: PoolId
-> Cardano.ShelleyVerificationKeyHashStakePool
toPoolIdHash (PoolId pId) =
SL.KeyHash $ Hash.UnsafeHash pId
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't our pool id be already the hashed version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right now it is hashed, so it goes wrapped with ledger data constructors. Below, there is version that hashes verification key. But because knownStakePools will return hashes we are not going to use it

(toStakingKeyHash accXPrv)
(toPoolIdHash poolId)
let unsigned =
mkUnsignedTx timeToLive ownedIns outs [registerCert, delegateCert]
Copy link
Member

Choose a reason for hiding this comment

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

👍

@KtorZ KtorZ force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from c43e8ef to cda3f3c Compare June 10, 2020 15:10
@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from 88ce777 to 9841048 Compare June 10, 2020 16:26
@paweljakubas paweljakubas marked this pull request as ready for review June 10, 2020 18:24
@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from 9c82939 to d4d3c04 Compare June 11, 2020 08:23
@paweljakubas
Copy link
Contributor Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 11, 2020
@paweljakubas paweljakubas requested review from Anviking and KtorZ June 11, 2020 08:25
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 11, 2020

try

Build failed

@paweljakubas paweljakubas force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from 3033671 to e3b889c Compare June 11, 2020 13:14
@KtorZ KtorZ force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from 98c69a3 to a719a4c Compare June 11, 2020 14:48
@KtorZ
Copy link
Member

KtorZ commented Jun 11, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 11, 2020
1722: Join stake pool shelley r=KtorZ a=paweljakubas

# Issue Number

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

# Overview

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

- [x] I have mocked stake pool layer and added joinStakePool to shelley api server
- [x] I have added first tests (now wallet id and no pool id) for stake pools in shelley
- [x] I have added impl for mkDelegationJoinTx
- [x] I have extended mkDelegationJoinTx by adding WalletDelegation 
- [x] I have added tests for joining/rejoining
- [x] I have impl quitting and added tests

# 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
-->


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

iohk-bors bot commented Jun 11, 2020

Build failed

@KtorZ
Copy link
Member

KtorZ commented Jun 11, 2020

bors r-

@KtorZ
Copy link
Member

KtorZ commented Jun 11, 2020

@KtorZ
Copy link
Member

KtorZ commented Jun 11, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 11, 2020
1722: Join stake pool shelley r=KtorZ a=paweljakubas

# Issue Number

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

# Overview

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

- [x] I have mocked stake pool layer and added joinStakePool to shelley api server
- [x] I have added first tests (now wallet id and no pool id) for stake pools in shelley
- [x] I have added impl for mkDelegationJoinTx
- [x] I have extended mkDelegationJoinTx by adding WalletDelegation 
- [x] I have added tests for joining/rejoining
- [x] I have impl quitting and added tests

# 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
-->


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

iohk-bors bot commented Jun 11, 2020

Build failed

@KtorZ KtorZ force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from 771de62 to c8ee7f2 Compare June 11, 2020 17:47
@KtorZ
Copy link
Member

KtorZ commented Jun 11, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 11, 2020
1722: Join stake pool shelley r=KtorZ a=paweljakubas

# Issue Number

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

# Overview

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

- [x] I have mocked stake pool layer and added joinStakePool to shelley api server
- [x] I have added first tests (now wallet id and no pool id) for stake pools in shelley
- [x] I have added impl for mkDelegationJoinTx
- [x] I have extended mkDelegationJoinTx by adding WalletDelegation 
- [x] I have added tests for joining/rejoining
- [x] I have impl quitting and added tests

# 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
-->


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

iohk-bors bot commented Jun 11, 2020

Build failed

@KtorZ KtorZ force-pushed the paweljakubas/1709/join-stake-pool-shelley branch from c8ee7f2 to b5d966e Compare June 12, 2020 06:56
@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Jun 12, 2020
The transaction submission command seems to occasionally fail on
macos, unable to open the socket.
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 12, 2020

try

Build failed

@rvl
Copy link
Contributor

rvl commented Jun 12, 2020

Retrying submitTx worked when testing locally...

bors try

iohk-bors bot added a commit that referenced this pull request Jun 12, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 12, 2020

try

Build failed

KtorZ added 2 commits June 12, 2020 11:38
Most messages from the node are actually redundant with the messages we
get from our own networking layer.
Pools are still registered on-chain beforehand, so we can still get to test a bunch of things.
@KtorZ
Copy link
Member

KtorZ commented Jun 12, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 12, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 5417c0a into master Jun 12, 2020
@iohk-bors iohk-bors bot deleted the paweljakubas/1709/join-stake-pool-shelley branch June 12, 2020 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants