Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Update to Lisk Core 1.0.0 API - Closes #341 #1032

Merged
merged 50 commits into from
May 15, 2018

Conversation

slaweet
Copy link
Contributor

@slaweet slaweet commented Jan 22, 2018

What was the problem?

Lisk Nano used Lisk Core 0.9.X API

How did I fix it?

Updated to Lisk Core 1.0.0 API

Changes include the following:

  • some endpoint names changed (e.g. /api/accounts/delegates to /api/votes)
  • each response now contains the main content in data object, before the name of the object depended on the endpoint (e.g. transactions, delegates)
  • account.secondSignature property was removed. account.secondPublicKey can be used instead
  • delegate. rate property was removed. delegate.rank can be used instead
  • delegate.address and delegate.publicKey was moved to delegate.account.address and delegate.account.publicKey

There were some issues in Lisk Core:

Every thing works completely with latest changes of lisk core witch contains in development branch, but there is no release for some of these features. So I had to suspend some of e2e tests.

-- infinite scroll in all lists are broken. there is a separated task to fix them #1065

How to test it?

  1. Checkout development branch of lisk core
  2. Then run lisk nano and check all features

Review checklist

@slaweet slaweet self-assigned this Jan 22, 2018
@slaweet slaweet changed the title Update lisk core to 1.0.0 - Closes #341 Update to Lisk Core 1.0.0 API - Closes #341 Jan 22, 2018
@slaweet slaweet force-pushed the 341-update-lisk-core-to-1-0-0 branch 3 times, most recently from e951c03 to c5a56cd Compare January 23, 2018 08:18
@slaweet slaweet force-pushed the 341-update-lisk-core-to-1-0-0 branch from c5a56cd to ca1e6fb Compare January 23, 2018 10:36
@slaweet slaweet force-pushed the 341-update-lisk-core-to-1-0-0 branch from ac5990a to 27f59fc Compare March 20, 2018 09:12
@fchavant fchavant force-pushed the 341-update-lisk-core-to-1-0-0 branch 2 times, most recently from c198b8c to 798c356 Compare March 27, 2018 13:49
@slaweet
Copy link
Contributor Author

slaweet commented May 14, 2018

I found the following issues:

  • Websocket connection to https://betanet.lisk.io throws 400
  • Delegates are not sorted by rank
  • Vote API error is not handled, results in: api_resource.js:65 Uncaught (in promise) Error: Status 409 : Failed to add vote, delegate "lisksnake" already voted for
  • Verify message should show a message "Message verified"/ "Message not verified" instead of "true"/ nothing
  • Extra slash at the end of custom node URL (e.g. https://betanet.lisk.io/) should allow me to log it.
  • The app fails to start with saved accounts. It throws: Error: APIClient requires nodes for initialization.

@yasharAyari you can fix them in this PR od create a follow-up.

Copy link
Contributor Author

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

One more thing:

  • all API calls used to dispatch event loadingStarted before and loadingFinished after to show/hide the loading bar.

if (!config.testnet && response.nethash !== netHashes.mainnet) {
config.nethash = response.nethash;
const getNethash = new Lisk.APIClient(config.nodes, config.nethash, {});
getNethash.node.getConstants().then((response) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getNethash is not describing what is stored in the variable, it is more something like apiClientInstance

Copy link
Contributor Author

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

In general loadingFinished should be not only in all then callbacks but also in catch callbacks. Otherwise, every time something fails, the loading bar stays forever.

@@ -24,15 +25,17 @@ const loginMiddleware = store => next => (action) => {
address,
};
const { activePeer } = action.data;

loadingStarted('loginMiddleware');
loadingFinished('loginMiddleware');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

loadingFinished looks like extra here

@webmaster128
Copy link
Contributor

webmaster128 commented May 15, 2018

Is this branch supposed to work with beta6 nodes? I get an error 400 when Nano tries to load http://node01.betanet.lisk.prolina.org:5000/api/transactions?senderIdOrRecipientId=6237734014301098166L&limit=25&offset=0&sort=timestamp%3Adesc

@slaweet
Copy link
Contributor Author

slaweet commented May 15, 2018

@webmaster128 That parameter will be fixed in beta7 LiskArchive/lisk-sdk#1920

Copy link
Contributor Author

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

Now it's great. Good job, Yashar.

I cannot approve the PR, because I opened it in the first place. Feel free to approve it yourself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants