Handle unconfirmed trx in wallet #24
Conversation
| public List<UtxoData> PendingAdd { get; set; } = new(); | ||
| public List<UtxoData> PendingRemove { get; set; } = new(); |
There was a problem hiding this comment.
We duplicate the data in storage, can't we just set it as a property on the UTXO data that is held in the address info utxo data?
There was a problem hiding this comment.
I tried that but it go more complicated, the duplication is negligible anyway
There was a problem hiding this comment.
Because of duplicates you need to pass both objects to the wallet operations and check each output you try to use to make sure it's not already used, this seems like you try to avoid a problem only to get it again down the line.
There was a problem hiding this comment.
The issue is more complicated, if we start to change the state that we receive form the indexer then I will have to look at the state when syncing, the proposed flag on the object will get overridden when we refresh but the utxo is not confirmed yet, though we could overcome that by changing the structure of how we store data.
…nputs and outputs
…ke founder spend)
|
this is now ready to review @DavidGershony |
src/Angor/Shared/WalletOperations.cs
Outdated
| foreach (var addressInfo in accountInfo.AddressesInfo.Concat(accountInfo.ChangeAddressesInfo)) | ||
| { | ||
| // find all spent inputs to mark them as spent | ||
| foreach (var utxoData in addressInfo.UtxoData) | ||
| { | ||
| foreach (var outPoint in inputs) |
There was a problem hiding this comment.
3 nested loops, isn't there a better way to get the utxos?
There was a problem hiding this comment.
Even if I use linq there will still be loops it will just look shorter.
To optimize here we will need to use dictionaries but I think this is not really needed
src/Angor/Shared/Models/Outpoint.cs
Outdated
| return new Outpoint { outputIndex = (int)outPoint.N, transactionId = outPoint.Hash.ToString() }; | ||
| } | ||
|
|
||
| public static Outpoint Create(string trxid, int index) |
There was a problem hiding this comment.
Shouldn't this just be a constructor?
There was a problem hiding this comment.
I think its simpler to understand as it is used in linq queries
| @@ -368,7 +316,16 @@ public async Task UpdateAccountInfoWithNewAddressesAsync(AccountInfo accountInfo | |||
| { | |||
| var addressInfoToDelete = accountInfo.AddressesInfo.SingleOrDefault(_ => _.Address == addressInfo.Address); | |||
| if (addressInfoToDelete != null) | |||
| { | |||
| //TODO need to update the indexer response with mempool utxo as well so it is always consistant | |||
| foreach (var utxoData in addressInfo.UtxoData.Where(x => x.InMempoolTransaction)) | |||
There was a problem hiding this comment.
I think this is a mistake it should be addressInfoToDelete.UtxoData.Where(x => x.InMempoolTransaction)
No description provided.