Skip to content

Add signal when transaction found in wallet, and filtering on wallet#102

Merged
dangershony merged 1 commit intomasterfrom
feature/merge-from-city-chain
Apr 24, 2020
Merged

Add signal when transaction found in wallet, and filtering on wallet#102
dangershony merged 1 commit intomasterfrom
feature/merge-from-city-chain

Conversation

@sondreb
Copy link
Copy Markdown
Member

@sondreb sondreb commented Apr 23, 2020

  • Add a signal when transactoin is found in wallet.
  • Add options to filter what is returned from Accounts, Transactions and Balances.

- Add a signal when transactoin is found in wallet.
- Add options to filter what is returned from Accounts,  Transactions and Balances.
// Start the syncing process from the block before the earliest transaction was seen.
this.walletSyncManager.SyncFromHeight(chainedHeader.Height - 1);
// Start the sync from the day before it was created.
this.walletSyncManager.SyncFromHeight(chainedHeader.Height);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why remove the -1?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It previously synced just minus 1 block, now it syncs minus 1 day, see 3rd line from above this line.

/// <summary>
/// The wallet name set by selectwallet method. This is static since the controller is a stateless type. This value should probably be cached by an injected service in the future.
/// </summary>
private static string CurrentWalletName;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should fix this now, it will be a fast fix maybe use as a hack put the field on the WallletManager just to avoid it being static?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah I realize this is on RPC, it less crucial for me so this is fine (if it does not break tests)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Doesn't break anything, and it is to give a custom support for multi-wallet in the RPC. It's a fairly important extra RPC method for anyone who relies on the RPC and have multiple wallets, but I don't think there are many that will ever use this. It is very useful when debugging and working with multiple wallets at the same time (normal wallet and special wallets).

Copy link
Copy Markdown
Member

@dangershony dangershony left a comment

Choose a reason for hiding this comment

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

Brilliant I suggest I run tests locally to be sure it all works

@dangershony dangershony merged commit 8e2fe77 into master Apr 24, 2020
@sondreb sondreb deleted the feature/merge-from-city-chain branch May 7, 2020 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants