Skip to content
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.

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

GetOrDownloadBlockAsync Refactor #2123

Merged
merged 6 commits into from Aug 20, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -131,7 +131,7 @@ public async Task TestServicesAsync(string networkString)
{
using (var cts = new CancellationTokenSource(TimeSpan.FromMinutes(3)))
{
var block = await walletService.GetOrDownloadBlockAsync(hash, cts.Token);
var block = await walletService.FetchBlockAsync(hash, cts.Token);
Assert.True(File.Exists(Path.Combine(blocksFolderPath, hash.ToString())));
Logger.LogInfo<P2pTests>($"Full block is downloaded: {hash}.");
}
@@ -328,7 +328,7 @@ public async Task InitializeAsync(CancellationToken cancel)
foreach (BlockState blockState in KeyManager.GetTransactionIndex())
{
var relevantTransactions = confirmedTransactions.Where(x => x.BlockHash == blockState.BlockHash).ToArray();
var block = await GetOrDownloadBlockAsync(blockState.BlockHash, cancel);
var block = await FetchBlockAsync(blockState.BlockHash, cancel);
await ProcessBlockAsync(blockState.BlockHeight, block, blockState.TransactionIndices, relevantTransactions);
}

@@ -415,7 +415,7 @@ private async Task ProcessFilterModelAsync(FilterModel filterModel, Cancellation
return;
}

Block currentBlock = await GetOrDownloadBlockAsync(filterModel.BlockHash, cancel); // Wait until not downloaded.
Block currentBlock = await FetchBlockAsync(filterModel.BlockHash, cancel); // Wait until not downloaded.

if (await ProcessBlockAsync(filterModel.BlockHeight, currentBlock))
{
@@ -586,41 +586,52 @@ public Node LocalBitcoinCoreNode
private set => _localBitcoinCoreNode = value;
}

/// <param name="hash">Block hash of the desired block, represented as a 256 bit integer.</param>
/// <exception cref="OperationCanceledException"></exception>
public async Task<Block> GetOrDownloadBlockAsync(uint256 hash, CancellationToken cancel)
public async Task<Block> FetchBlockAsync(uint256 hash, CancellationToken cancel)
{
Block block = await GetBlockFromFileAsync(hash, cancel);
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 16, 2019

Collaborator

GetBlockFromFileAsync -> TryGetBlockFromFileAsync

if (block == null)
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

is null can be used.

{
block = await DownloadBlockAsync(hash, cancel);
}
return block;
}

/// <param name="hash">Block hash of the desired block, represented as a 256 bit integer.</param>
/// <exception cref="OperationCanceledException"></exception>
private async Task<Block> GetBlockFromFileAsync(uint256 hash, CancellationToken cancel)
{
// Try get the block
Block block = null;
using (await BlockFolderLock.LockAsync())
{
var encoder = new HexEncoder();
foreach (var filePath in Directory.EnumerateFiles(BlocksFolderPath))
var filePath = Path.Combine(BlocksFolderPath, encoder.EncodeData(hash.ToBytes()));
if (Directory.Exists(filePath))
{
var fileName = Path.GetFileName(filePath);
if (!encoder.IsValid(fileName))
try
{
Logger.LogTrace<WalletService>($"Filename is not a hash: {fileName}.");
continue;
var blockBytes = await File.ReadAllBytesAsync(filePath);
block = Block.Load(blockBytes, Synchronizer.Network);
}

if (hash == new uint256(fileName))
catch (Exception)
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 16, 2019

Collaborator

Don't leave empty catch. Maybe log the error and delete the file?

{
var blockBytes = await File.ReadAllBytesAsync(filePath);
try
{
return Block.Load(blockBytes, Synchronizer.Network);
}
catch (Exception)
{
// In case the block file is corrupted we get an EndOfStreamException exception
// Ignore any error and continue by re-downloading the block.
break;
}
// In case the block file is corrupted and we get an EndOfStreamException exception
// Ignore any error and continue to re-downloading the block.
}
}
}

cancel.ThrowIfCancellationRequested();
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 16, 2019

Collaborator

No need to throw here. The function already finished. Maybe throw outside the block and/or can't ReadAllBytesAsync get a cancellation token??


// Download the block
return block;
}

/// <param name="hash">Block hash of the desired block, represented as a 256 bit integer.</param>
/// <exception cref="OperationCanceledException"></exception>
private async Task<Block> DownloadBlockAsync(uint256 hash, CancellationToken cancel)
{
Block block = null;
try
{
@@ -633,125 +644,44 @@ public async Task<Block> GetOrDownloadBlockAsync(uint256 hash, CancellationToken
try
{
// Try to get block information from local running Core node first.
try
{
if (LocalBitcoinCoreNode is null || !LocalBitcoinCoreNode.IsConnected && Network != Network.RegTest) // If RegTest then we're already connected do not try again.
{
DisconnectDisposeNullLocalBitcoinCoreNode();
using (var handshakeTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancel))
{
handshakeTimeout.CancelAfter(TimeSpan.FromSeconds(10));
var nodeConnectionParameters = new NodeConnectionParameters()
{
ConnectCancellation = handshakeTimeout.Token,
IsRelay = false,
UserAgent = $"/Wasabi:{Constants.ClientVersion}/"
};

// If an onion was added must try to use Tor.
// onlyForOnionHosts should connect to it if it's an onion endpoint automatically and non-Tor endpoints through clearnet/localhost
if (Synchronizer.WasabiClient.TorClient.IsTorUsed)
{
nodeConnectionParameters.TemplateBehaviors.Add(new SocksSettingsBehavior(Synchronizer.WasabiClient.TorClient.TorSocks5EndPoint, onlyForOnionHosts: true, networkCredential: null, streamIsolation: false));
}

var localEndPoint = ServiceConfiguration.BitcoinCoreEndPoint;
var localNode = await Node.ConnectAsync(Network, localEndPoint, nodeConnectionParameters);
try
{
Logger.LogInfo<WalletService>($"TCP Connection succeeded, handshaking...");
localNode.VersionHandshake(Constants.LocalNodeRequirements, handshakeTimeout.Token);
var peerServices = localNode.PeerVersion.Services;

//if (!peerServices.HasFlag(NodeServices.Network) && !peerServices.HasFlag(NodeServices.NODE_NETWORK_LIMITED))
//{
// throw new InvalidOperationException($"Wasabi cannot use the local node because it does not provide blocks.");
//}

Logger.LogInfo<WalletService>($"Handshake completed successfully.");

if (!localNode.IsConnected)
{
throw new InvalidOperationException($"Wasabi could not complete the handshake with the local node and dropped the connection.{Environment.NewLine}" +
$"Probably this is because the node does not support retrieving full blocks or segwit serialization.");
}
LocalBitcoinCoreNode = localNode;
}
catch (OperationCanceledException) when (handshakeTimeout.IsCancellationRequested)
{
Logger.LogWarning<Node>($"Wasabi could not complete the handshake with the local node. Probably Wasabi is not whitelisted by the node.{Environment.NewLine}" +
$"Use \"whitebind\" in the node configuration. (Typically whitebind=127.0.0.1:8333 if Wasabi and the node are on the same machine and whitelist=1.2.3.4 if they are not.)");
throw;
}
}
}
block = await DownloadBlockFromLocalNodeAsync(hash, cancel);

Block blockFromLocalNode = null;
// Should timeout faster. Not sure if it should ever fail though. Maybe let's keep like this later for remote node connection.
using (var cts = new CancellationTokenSource(TimeSpan.FromSeconds(64)))
{
blockFromLocalNode = await LocalBitcoinCoreNode.DownloadBlockAsync(hash, cts.Token);
}

if (!blockFromLocalNode.Check())
{
throw new InvalidOperationException($"Disconnected node, because invalid block received!");
}

block = blockFromLocalNode;
Logger.LogInfo<WalletService>($"Block acquired from local P2P connection: {hash}");
break;
}
catch (Exception ex)
if (block != null)
{
block = null;
DisconnectDisposeNullLocalBitcoinCoreNode();

if (ex is SocketException)
{
Logger.LogTrace<WalletService>("Did not find local listening and running full node instance. Trying to fetch needed block from other source.");
}
else
{
Logger.LogWarning<WalletService>(ex);
}
break;
}
cancel.ThrowIfCancellationRequested();

// If no connection, wait then continue.
// If no connection, wait, then continue.
while (Nodes.ConnectedNodes.Count == 0)
{
await Task.Delay(100);
}

// Select a random node we are connected to.
Node node = Nodes.ConnectedNodes.RandomElement();
if (node == default(Node))
{
await Task.Delay(100);
continue;
}

if (!node.IsConnected && !(Synchronizer.Network != Network.RegTest))
if (node == default(Node) && !node.IsConnected && !(Synchronizer.Network != Network.RegTest))
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

Why not && Synchronizer.Network == Network.RegTest?

{
await Task.Delay(100);
continue;
}

// Download block from selected node.
try
{
using (var cts = new CancellationTokenSource(TimeSpan.FromSeconds(RuntimeParams.Instance.NetworkNodeTimeout))) // 1/2 ADSL 512 kbit/s 00:00:32
{
block = await node.DownloadBlockAsync(hash, cts.Token);
}

// Validate block
if (!block.Check())
{
Logger.LogInfo<WalletService>($"Disconnected node: {node.RemoteSocketAddress}, because invalid block received.");
node.DisconnectAsync("Invalid block received.");
continue;
}

if (Nodes.ConnectedNodes.Count > 1) // So to minimize risking missing unconfirmed transactions.
if (Nodes.ConnectedNodes.Count > 1) // To minimize risking missing unconfirmed transactions.
{
Logger.LogInfo<WalletService>($"Disconnected node: {node.RemoteSocketAddress}. Block downloaded: {block.GetHash()}");
node.DisconnectAsync("Thank you!");
@@ -778,7 +708,7 @@ public async Task<Block> GetOrDownloadBlockAsync(uint256 hash, CancellationToken
continue;
}

break; // If got this far break, then we have the block, it's valid. Break.
break; // If got this far, then we have the block and it's valid. Break.
}
catch (Exception ex)
{
@@ -802,6 +732,97 @@ public async Task<Block> GetOrDownloadBlockAsync(uint256 hash, CancellationToken
return block;
}

private async Task<Block> DownloadBlockFromLocalNodeAsync(uint256 hash, CancellationToken cancel)
{
try
{
if (LocalBitcoinCoreNode is null || !LocalBitcoinCoreNode.IsConnected && Network != Network.RegTest) // If RegTest then we're already connected do not try again.
{
DisconnectDisposeNullLocalBitcoinCoreNode();
using (var handshakeTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancel))
{
handshakeTimeout.CancelAfter(TimeSpan.FromSeconds(10));
var nodeConnectionParameters = new NodeConnectionParameters()
{
ConnectCancellation = handshakeTimeout.Token,
IsRelay = false,
UserAgent = $"/Wasabi:{Constants.ClientVersion}/"
};

// If an onion was added must try to use Tor.
// onlyForOnionHosts should connect to it if it's an onion endpoint automatically and non-Tor endpoints through clearnet/localhost
if (Synchronizer.WasabiClient.TorClient.IsTorUsed)
{
nodeConnectionParameters.TemplateBehaviors.Add(new SocksSettingsBehavior(Synchronizer.WasabiClient.TorClient.TorSocks5EndPoint, onlyForOnionHosts: true, networkCredential: null, streamIsolation: false));
}

var localEndPoint = ServiceConfiguration.BitcoinCoreEndPoint;
var localNode = await Node.ConnectAsync(Network, localEndPoint, nodeConnectionParameters);
try
{
Logger.LogInfo<WalletService>("TCP Connection succeeded, handshaking...");
localNode.VersionHandshake(Constants.LocalNodeRequirements, handshakeTimeout.Token);
var peerServices = localNode.PeerVersion.Services;

//if (!peerServices.HasFlag(NodeServices.Network) && !peerServices.HasFlag(NodeServices.NODE_NETWORK_LIMITED))
//{
// throw new InvalidOperationException($"Wasabi cannot use the local node because it does not provide blocks.");
//}

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

Should this commented code be removed?

This comment has been minimized.

Copy link
@benthecarman

benthecarman Aug 16, 2019

Author Collaborator

Was there beforehand, most of this is just copy-pasting

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 18, 2019

Collaborator

It can go I think. @lontivero ?


Logger.LogInfo<WalletService>($"Handshake completed successfully.");

if (!localNode.IsConnected)
{
throw new InvalidOperationException($"Wasabi could not complete the handshake with the local node and dropped the connection.{Environment.NewLine}" +
$"Probably this is because the node does not support retrieving full blocks or segwit serialization.");
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

$ can be removed.

}
LocalBitcoinCoreNode = localNode;
}
catch (OperationCanceledException) when (handshakeTimeout.IsCancellationRequested)
{
Logger.LogWarning<Node>($"Wasabi could not complete the handshake with the local node. Probably Wasabi is not whitelisted by the node.{Environment.NewLine}" +
$"Use \"whitebind\" in the node configuration. (Typically whitebind=127.0.0.1:8333 if Wasabi and the node are on the same machine and whitelist=1.2.3.4 if they are not.)");
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

$ can be removed.

throw;
}
}
}

// Get Block from local node
Block blockFromLocalNode = null;
// Should timeout faster. Not sure if it should ever fail though. Maybe let's keep like this later for remote node connection.
using (var cts = new CancellationTokenSource(TimeSpan.FromSeconds(64)))
{
blockFromLocalNode = await LocalBitcoinCoreNode.DownloadBlockAsync(hash, cts.Token);
}

// Validate retrieved block
if (!blockFromLocalNode.Check())
{
throw new InvalidOperationException("Disconnected node, because invalid block received!");
}

// Retrieved block from local node and block is valid
Logger.LogInfo<WalletService>($"Block acquired from local P2P connection: {hash}");
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@yahiheb

yahiheb Aug 16, 2019

Collaborator

Add . at the end

return blockFromLocalNode;
}
catch (Exception ex)
{
DisconnectDisposeNullLocalBitcoinCoreNode();

if (ex is SocketException)
{
Logger.LogTrace<WalletService>("Did not find local listening and running full node instance. Trying to fetch needed block from other source.");
}
else
{
Logger.LogWarning<WalletService>(ex);
}
}
cancel.ThrowIfCancellationRequested();
This conversation was marked as resolved by benthecarman

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 16, 2019

Collaborator

No need to throw at the end of the function (same reasons as above.)


return null;
}
This conversation was marked as resolved by nopara73

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 16, 2019

Collaborator

This whole part is just copypaste, right?

This comment has been minimized.

Copy link
@benthecarman

benthecarman Aug 16, 2019

Author Collaborator

Yes


private void DisconnectDisposeNullLocalBitcoinCoreNode()
{
if (LocalBitcoinCoreNode != null)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.