Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@valentinewallace
Copy link
Contributor

Closes #1006.

@valentinewallace valentinewallace requested a review from tanx March 13, 2019 02:37
log.error('Getting limbo balance failed', err);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

store.limboBalanceSatoshis should be done in getPendingChannels of channel actions as that grpc api is already being polled. Doing it here would result in the api being polled twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

pendingBalanceSatoshis +
channelBalanceSatoshis +
limboBalanceSatoshis
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Although store.limboBalanceSatoshis is set in channel actions I would also compute this here 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm confused what this means. What is being computed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just saying it makes sense to use the ComputedWallet module here. So no changes needed :)

expect(store.totalBalanceSatoshis, 'to equal', 100010001);
expect(store.totalBalanceLabel, 'to match', /1[,.]000[,.]100[,.]01/);
expect(store.totalBalanceSatoshis, 'to equal', 100010101);
expect(store.totalBalanceLabel, 'to match', /1[,.]000[,.]101[,.]01/);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test 👍

pendingBalanceSatoshis +
channelBalanceSatoshis +
limboBalanceSatoshis
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just saying it makes sense to use the ComputedWallet module here. So no changes needed :)

@tanx tanx merged commit 0666f01 into master Mar 14, 2019
@tanx tanx deleted the limbo-balance branch March 14, 2019 03:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants