Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Supporting internal -> external transactions #1143

Merged
merged 21 commits into from
Sep 10, 2019

Conversation

T-Dnzt
Copy link

@T-Dnzt T-Dnzt commented Aug 26, 2019

Issue/Task Number: #1086

Overview

This PR adds the support for internal to external, in the fastest possible way. The code isn't super clean, let's be honest, but it works.

Changes

  • Allow the transfer from internal wallets to external addresses.

Implementation Details

Add a new status to local ledger transactions, that's defaulting to confirmed. The eWallet can specify a transaction as being pending, which means it's included into the calculations, but can be turned into a failed transaction, which will not be counted.

@T-Dnzt T-Dnzt changed the base branch from master to 1087-tx-listener August 26, 2019 04:43
@T-Dnzt T-Dnzt requested a review from unnawut August 26, 2019 04:43
@T-Dnzt T-Dnzt self-assigned this Aug 26, 2019
Copy link
Contributor

@unnawut unnawut left a comment

Choose a reason for hiding this comment

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

Looks pretty much the same pattern as the existing blockchain transaction code so...

apps/local_ledger_db/lib/local_ledger_db/entry.ex Outdated Show resolved Hide resolved
@unnawut unnawut changed the base branch from 1087-tx-listener to eth-blockchain September 2, 2019 07:01
@unnawut
Copy link
Contributor

unnawut commented Sep 2, 2019

@T-Dnzt Can you have a look at my changes? Looking commit by commit will be much easier...

@mederic-p
Copy link
Contributor

Missing: Handle cached balance for pending transaction that turn to failed.

@T-Dnzt
Copy link
Author

T-Dnzt commented Sep 4, 2019

Looks good, as Mederic pointed out, we need to re-calculate the cached balance (from the beginning) when a ledger transaction is switched to failed.

@T-Dnzt
Copy link
Author

T-Dnzt commented Sep 9, 2019

Can't approve because I made the PR, but LGTM 👍

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.

None yet

3 participants