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

Implement storing BTC transactions data - Closes #1945 #1955

Merged
merged 37 commits into from May 8, 2019

Conversation

Projects
None yet
4 participants
@slaweet
Copy link
Member

commented Apr 25, 2019

What issue have I solved?

#1945

How have I implemented/fixed it?

I changed getTransactions API util to be token-agnostic and to get token-specific getTransactions (BTC or LSK) based on whether the passed address parameter is BTC or LSK address.

Analogically, I changed getSignleTransaction to be token-agnostic based on whether the passed id parameter is BTC or LSK id.

Note that currently it is based on branch 1946-logic-for-btc-peer. After that branch PR is merged, I will change the base branch of this PR to development

How has this been tested?

  1. Go to https://jenkins.lisk.io/test/lisk-hub/PR-1955/
  2. log it to testnet
  3. Set localStorage.setItem('btc', true);
  4. Go to https://jenkins.lisk.io/test/lisk-hub/PR-1955/#/explorer/accounts/n3aZt7uZhnBeC9quq6btKyC8qXvskEiE1B
  5. see there are BTC transactions loaded.
  6. Go to https://jenkins.lisk.io/test/lisk-hub/PR-1955/#/explorer/transactions/553787aa91a55e0385beb81a0a549f02cb3be8492b452fe8b09c1729165a8362
  7. see there a BTC transaction details.

Note that there will be another ticket to make the page show everything properly.

Review checklist

@slaweet slaweet self-assigned this Apr 25, 2019

@slaweet slaweet requested review from osvaldovega and yasharAyari Apr 25, 2019

@osvaldovega
Copy link
Contributor

left a comment

I think just on little thing but everything looks good

Show resolved Hide resolved src/utils/api/lsk/transactions.js Outdated

@slaweet slaweet force-pushed the 1945-storing-btc-transactions branch from 346fb3b to 8689326 Apr 26, 2019

@slaweet slaweet force-pushed the 1946-logic-for-btc-peer branch from a1a72aa to 342ab79 Apr 29, 2019

@slaweet slaweet force-pushed the 1945-storing-btc-transactions branch from cc06a25 to e5eab8f Apr 29, 2019

slaweet added some commits Apr 23, 2019

♻️ Update btc transactions utils
... to be compatible with the latest bitcoin bridge
🐛 Prevent accountVisual breaking whole page
... if non-LSK address is passed to it

@slaweet slaweet changed the base branch from 1946-logic-for-btc-peer to development Apr 29, 2019

@slaweet slaweet marked this pull request as ready for review Apr 29, 2019

🐛 Enable the new network action,
... as it is needed for transaction utils to work

@slaweet slaweet requested a review from massao Apr 30, 2019

@yasharAyari
Copy link
Member

left a comment

Sign in page doesn't work after the new changes.

Show resolved Hide resolved src/utils/api/transactions.js Outdated
@massao
Copy link
Contributor

left a comment

Just a small question, other than that, everything seems ok.

Show resolved Hide resolved src/utils/api/btc/transactions.js

slaweet added some commits Apr 30, 2019

🐛 Allow liskAPIClient in getSingleTransaction
... to make it work in search actions

slaweet added some commits Apr 30, 2019

@osvaldovega
Copy link
Contributor

left a comment

🎉 perfect Vit

waiting for rest of the team for approval

@slaweet slaweet requested a review from yasharAyari May 3, 2019

@slaweet

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

There is a number of TODO comments referring to things that need to be done in the future when enabling the BTC feature to the end user.
For the future reference (as suggested by @osvaldovega ), here's a command to find them all:

git grep TODO | grep -i btc
Show resolved Hide resolved src/actions/account.js
Show resolved Hide resolved src/actions/search.js

@slaweet slaweet requested a review from yasharAyari May 3, 2019

@slaweet slaweet merged commit 6f4396f into development May 8, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
security/snyk - package.json (LiskHQ) No manifest changes detected

@slaweet slaweet added the ready label May 8, 2019

@slaweet slaweet deleted the 1945-storing-btc-transactions branch May 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.