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.

Sign up for GitHub

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

Jump to bottom

refactor(account): keep all accounts in memory #174

Merged

lucasfernog merged 31 commits into develop from fix/account-storage

Dec 18, 2020

Contributor

lucasfernog commented Dec 16, 2020

Description of change

Instead of loading the account from storage and saving on each operation, we'll keep the account references stored on the AccountManager struct, wrapped on Arc<RwLock<>> for thread safety. This prevents race conditions between e.g. the polling system and the transfer API, where account data can be lost.

Type of change

Refactor

How the change has been tested

Unit tests + faucet.

Change checklist

I have followed the contribution guidelines for this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have checked that new and existing unit tests pass locally with my changes

lucasfernog added 14 commits

December 15, 2020 14:55


          feat(manager): initial work on account guard

6c3e3ec


          feat(manager): adjust get_account to use the store

eef5039


          Merge branch 'develop' into fix/account-storage

832166d


          refactor(lib): use AccountGuard instead of Account

3790e30


          fix(lib): tests and examples compilation

9a7685a


          fix(lib): tests

e6ce170


          chore(transfer): return message instead of wrapper

ab1fb18


          refactor(lib): update nodejs binding with latest account refactor

ca93f15


          feat(account): add bridge methods to the guard

184d2d5


          Merge branch 'develop' into fix/account-storage

c8b93d0


          fix(sync): deadlocks

b11b3f8


          feat(bindings): add loadAccounts method

6919b14


          chore(lib): cleanup

3cfb6a3


          chore(transfer): lock write only when needed

f8aa72c

zesterer reviewed

View reviewed changes

bindings/node/native/src/lib.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account/mod.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account/mod.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account/mod.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/actor/mod.rs Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/monitor.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/monitor.rs Outdated Show resolved Hide resolved

Alex6323 reviewed

View reviewed changes

bindings/node/native/src/classes/account_manager/mod.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/monitor.rs Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/actor/mod.rs Show resolved Hide resolved

Alex6323 reviewed

View reviewed changes

bindings/node/native/src/classes/synced_account/mod.rs Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

Alex6323 reviewed

View reviewed changes

bindings/node/native/src/lib.rs Outdated Show resolved Hide resolved

Alex6323 reviewed

View reviewed changes

src/account/mod.rs Show resolved Hide resolved

lucasfernog added 15 commits

December 16, 2020 15:53


          chore(account): rename AccountGuard to AccountHandle

6aa0430


          chore(lib): inline usage of account handle where appropriate

92f6d95


          chore(lib): rename AccountHandle variables to account_handle

1d90529


          perf(manager): get accounts by alias method

9f5ed37


          refactor(lib): use tokio's RwLock on AccountHandle

b9c51b6


          refactor(lib): use tokio's RwLock on Client

d7482fb


          refactor(lib): use tokio's RwLock on accountmanager accounts

828cc23


          feat(manager): use polling system with tokio instead of thread::spawn

9c0f561


          chore(bindings): rename variable

bbc280c


          refactor(account): perform mutable operations with do_mut

285a326


          refactor(manager): remove load_accounts and start_background_sync

2a00b38


          refactor(manager): change get_account_by_alias' alias type

95fdca3


          fix(tests): deadlock

09451b8


          fix(tests): actor spawn

c94b8ec


          fix(tests): doc tests

8e94f8e

Contributor Author

lucasfernog commented Dec 17, 2020

Next steps (separate PRs):

remove anyhow usage
add AccountManager builder
stronghold refactor

zesterer reviewed

View reviewed changes

bindings/node/native/src/lib.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

zesterer reviewed

View reviewed changes

src/account_manager.rs Outdated Show resolved Hide resolved

lucasfernog added 2 commits

December 18, 2020 09:53


          code review changes

36c1140


          fix(tests): deadlock

63eb171

zesterer approved these changes

View reviewed changes

rajivshah3 approved these changes

View reviewed changes

lucasfernog merged commit f56e1cf into develop

lucasfernog deleted the fix/account-storage branch

December 18, 2020 16:27

SuharH mentioned this pull request

Account alias already exist error when there is no account #1245

Closed

2 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment