Skip to content

Commit

Permalink
fix(wallet): correctly deal with new coinbase transactions for the sa…
Browse files Browse the repository at this point in the history
…me height (#3151)

## Description
A bug was observed that now and then a Coinbase Transaction would not conclude its monitoring with this error:
`ValueNotFound(CompletedTransaction(15645503741743694020))`

This occurred when due to a small network reorg the miner would request a block for the same height. The new request would often have a different amount of fees which would mean this transaction needs to be cancelled in favour of the new transaction. However, with the transaction being cancelled the coinbase monitoring task will come around to checking the DB if the transaction is still there so it can continue to poll the base node and will get the ValueNotFound error. This is correct behaviour so should not have been logged as an error. The error case also means that the TransactionCancelled event is never fired which resulted in the console wallet UI not updating the state of that transaction which left it in the transaction list erroneous.

So this PR handles that “Error” properly and sends the event before ending the coinbase monitoring task.

## How Has This Been Tested?
unit tests

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
* [x] I'm merging against the `development` branch.
* [x] I have squashed my commits into a single commit.
  • Loading branch information
aviator-app[bot] committed Aug 4, 2021
2 parents 5de9252 + f864e20 commit 564ef5a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
5 changes: 5 additions & 0 deletions base_layer/wallet/src/output_manager_service/service.rs
Expand Up @@ -663,6 +663,11 @@ where TBackend: OutputManagerBackend + 'static
fees: MicroTari,
block_height: u64,
) -> Result<Transaction, OutputManagerError> {
debug!(
target: LOG_TARGET,
"Building coinbase transaction for block_height {} with TxId: {}", block_height, tx_id
);

let (spending_key, script_key) = self
.resources
.master_key_manager
Expand Down
Expand Up @@ -121,17 +121,23 @@ where TBackend: TransactionBackend + 'static
let completed_tx = match self.resources.db.get_completed_transaction(self.tx_id).await {
Ok(tx) => tx,
Err(e) => {
error!(
info!(
target: LOG_TARGET,
"Cannot find Completed Transaction (TxId: {}) referred to by this Coinbase Monitoring \
Protocol: {:?}",
self.tx_id,
e
"Cannot find Coinbase Transaction (TxId: {}) likely due to being cancelled: {}", self.tx_id, e
);
return Err(TransactionServiceProtocolError::new(
self.tx_id,
TransactionServiceError::TransactionDoesNotExistError,
));
let _ = self
.resources
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(self.tx_id)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
"Error sending event, usually because there are no subscribers: {:?}",
e
);
e
});
return Ok(self.tx_id);
},
};
debug!(
Expand Down Expand Up @@ -332,9 +338,7 @@ where TBackend: TransactionBackend + 'static
}
}
}
result = self.query_coinbase_transaction(
signature.clone(), completed_tx.clone(), &mut client
).fuse() => {
result = self.query_coinbase_transaction(signature.clone(), completed_tx.clone(), &mut client).fuse() => {
let (coinbase_kernel_found, metadata) = match result {
Ok(r) => r,
_ => (false, None),
Expand Down

0 comments on commit 564ef5a

Please sign in to comment.