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

Additional tweaks with regards to static address construction via the API #2318

Merged

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Nov 12, 2020

Issue Number

ADP-461

Overview

  • 9b362af
    📍 return non-extended public key (without chain code) as 'ApiVerificationKey'
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

  • 3b00583
    📍 revise API documentation regarding address construction from script

    • give more details about how scripts are constructed
    • move description around so they show up correctly in ReDoc.
    • move address endpoints into the 'Address' group and remove the 'Utils' group

Comments

TODO: use public key in script construction instead of hashes, and do the hashing internally.

@KtorZ KtorZ added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Nov 12, 2020
@KtorZ KtorZ self-assigned this Nov 12, 2020
@KtorZ KtorZ force-pushed the KtorZ/ADP-461/additional-tweaks-address-construction branch from 3b00583 to 4c4c963 Compare November 12, 2020 16:13
Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

LGTM! Left some suggestions. Very fond of infographic you included!

…onKey'

  The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.
  - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group
@KtorZ KtorZ force-pushed the KtorZ/ADP-461/additional-tweaks-address-construction branch from 7462fb4 to 5c8cabe Compare November 13, 2020 09:02
@KtorZ
Copy link
Member Author

KtorZ commented Nov 13, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 13, 2020
2318: Additional tweaks with regards to static address construction via the API r=KtorZ a=KtorZ

# Issue Number

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

ADP-461

# Overview

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

- 9b362af
  📍 **return non-extended public key (without chain code) as 'ApiVerificationKey'**
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

- 3b00583
  📍 **revise API documentation regarding address construction from script**
    - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group

# Comments

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

TODO: use public key in script construction instead of hashes, and do the hashing internally. 

<!-- 
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: KtorZ <[email protected]>
@@ -998,23 +999,19 @@ spec = describe "SHELLEY_WALLETS" $ do
matrix =
Copy link
Contributor

Choose a reason for hiding this comment

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

Idea for integration test:

  • create empty shelley wallet that has {walletId}
  • let paymentKey = GET /wallets/{walletId}/keys/utxo_external/0
  • let stakeKey = GET /wallets/{walletId}/keys/mutable_account/0
  • Construct address:
POST /addresses
{payment: ${paymentKey}, stake: ${stakeKey}}
  • verify that this address is first address on {walletId}

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like it would work 👍

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 13, 2020

Build failed:


Failures:
--
  |  
  | src/Test/Integration/Scenario/API/Shelley/Transactions.hs:642:10:
  | 1) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_01 - Pending transaction expiry
  | predicate failed on: SlotNo 75
  | expected expiry: SlotNo 37597
  | actual expiry: SlotNo 37672
  |  
  | To rerun use: --match "/API Specifications/SHELLEY_TRANSACTIONS/TRANS_TTL_01 - Pending transaction expiry/"
  |  
  | src/Test/Integration/Scenario/API/Shelley/Transactions.hs:676:10:
  | 2) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_02 - Custom transaction expiry
  | predicate failed on: SlotNo 76
  | expected expiry: SlotNo 1874
  | actual expiry: SlotNo 1950
  |  
  | To rerun use: --match "/API Specifications/SHELLEY_TRANSACTIONS/TRANS_TTL_02 - Custom transaction expiry/"
  |  
  | Randomized with seed 1701310684

#2295

@Anviking
Copy link
Member

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 13, 2020
2318: Additional tweaks with regards to static address construction via the API r=Anviking a=KtorZ

# Issue Number

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

ADP-461

# Overview

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

- 9b362af
  📍 **return non-extended public key (without chain code) as 'ApiVerificationKey'**
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

- 3b00583
  📍 **revise API documentation regarding address construction from script**
    - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group

# Comments

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

TODO: use public key in script construction instead of hashes, and do the hashing internally. 

<!-- 
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: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 13, 2020

Build failed:

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:433:5:
  1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_QUIT_02 - Passphrase must be correct to quit
       uncaught exception: RequestException
       DecodeFailure "{\"code\":\"network_unreachable\",\"message\":\"The node backend is unreachable at the moment. Trying again in a bit might work.\"}"

#2320

@KtorZ
Copy link
Member Author

KtorZ commented Nov 16, 2020

bors retry

iohk-bors bot added a commit that referenced this pull request Nov 16, 2020
2318: Additional tweaks with regards to static address construction via the API r=Anviking a=KtorZ

# Issue Number

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

ADP-461

# Overview

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

- 9b362af
  📍 **return non-extended public key (without chain code) as 'ApiVerificationKey'**
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

- 3b00583
  📍 **revise API documentation regarding address construction from script**
    - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group

# Comments

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

TODO: use public key in script construction instead of hashes, and do the hashing internally. 

<!-- 
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: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2020

Build failed:

      src/Test/Integration/Scenario/API/Shelley/Transactions.hs:642:10:
      1) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_01 - Pending transaction expiry
           predicate failed on: SlotNo 64
           expected expiry: SlotNo 37456
           actual expiry: SlotNo 37520

      src/Test/Integration/Scenario/API/Shelley/Transactions.hs:676:10:
      2) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_02 - Custom transaction expiry
           predicate failed on: SlotNo 64
           expected expiry: SlotNo 1666
           actual expiry: SlotNo 1730

#2295

@KtorZ
Copy link
Member Author

KtorZ commented Nov 16, 2020

bors retry

iohk-bors bot added a commit that referenced this pull request Nov 16, 2020
2318: Additional tweaks with regards to static address construction via the API r=Anviking a=KtorZ

# Issue Number

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

ADP-461

# Overview

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

- 9b362af
  📍 **return non-extended public key (without chain code) as 'ApiVerificationKey'**
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

- 3b00583
  📍 **revise API documentation regarding address construction from script**
    - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group

# Comments

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

TODO: use public key in script construction instead of hashes, and do the hashing internally. 

<!-- 
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: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2020

Build failed:

  1) API Specifications, BYRON_MIGRATIONS, BYRON_MIGRATE_01 -  migrate a big wallet requiring more than one tx
       While verifying (Status {statusCode = 200, statusMessage = "OK"},Right (ApiUtxoStatistics {total = Quantity {getQuantity = 939993308496}, scale = ApiT {getApiT = Log10}, distribution = fromList [(10,0),(100,0),(1000,0),(10000,0),(100000,0),(1000000,0),(10000000,0),(100000000,0),(1000000000,0),(10000000000,94),(100000000000,0),(1000000000000,0),(10000000000000,0),(100000000000000,0),(1000000000000000,0),(10000000000000000,0),(45000000000000000,0)]}))
       expected: Just 100
        but got: Just 94

#2207

@KtorZ
Copy link
Member Author

KtorZ commented Nov 16, 2020

bors retry

iohk-bors bot added a commit that referenced this pull request Nov 16, 2020
2318: Additional tweaks with regards to static address construction via the API r=Anviking a=KtorZ

# Issue Number

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

ADP-461

# Overview

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

- 9b362af
  📍 **return non-extended public key (without chain code) as 'ApiVerificationKey'**
    The chain code is useless because there's no further derivation that can happen at this stage. Plus, it does make it harder to re-use the output from the 'getWalletKey' endpoint into others because one needs to decode the bech32 string and strip away the chain code.

- 3b00583
  📍 **revise API documentation regarding address construction from script**
    - give more details about how scripts are constructed
  - move description around so they show up correctly in ReDoc.
  - move address endpoints into the 'Address' group and remove the 'Utils' group

# Comments

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

TODO: use public key in script construction instead of hashes, and do the hashing internally. 

<!-- 
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: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2020

Build failed:


1) API Specifications, SHELLEY_TRANSACTIONS, TRANS_TTL_01 - Pending transaction expiry
           predicate failed on: SlotNo 71
           expected expiry: SlotNo 37466
           actual expiry: SlotNo 37537

#2295

@KtorZ
Copy link
Member Author

KtorZ commented Nov 16, 2020

bors retry

please.

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 16, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit 602244c into master Nov 16, 2020
@iohk-bors iohk-bors bot deleted the KtorZ/ADP-461/additional-tweaks-address-construction branch November 16, 2020 18:11
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