Skip to content

Support bech32 addresses in rpc methods #211

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

Merged
dangershony merged 1 commit into from
Sep 2, 2020
Merged

Support bech32 addresses in rpc methods #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/Features/Blockcore.Features.Wallet/Api/Controllers/WalletRPCController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public async Task<uint256> SendTransactionAsync(string hex)
/// Uses the first wallet and account.
/// </summary>
/// <param name="account">Parameter is deprecated.</param>
/// <param name="addressType">Address type, currently only 'legacy' is supported.</param>
/// <param name="addressType">Address type, currently only 'legacy' and 'bech32' is supported.</param>
/// <returns>The new address.</returns>
[ActionName("getnewaddress")]
[ActionDescription("Returns a new wallet address for receiving payments.")]
Expand All @@ -178,26 +178,28 @@ public NewAddressModel GetNewAddress(string account = "", string addressType = "

if (!string.IsNullOrEmpty(addressType))
{
// Currently segwit and bech32 addresses are not supported.
if (!addressType.Equals("legacy", StringComparison.InvariantCultureIgnoreCase))
throw new RPCServerException(RPCErrorCode.RPC_METHOD_NOT_FOUND, "Only address type 'legacy' is currently supported.");
// Currently segwit addresses are not supported.
Copy link
Copy Markdown
Member

@dangershony dangershony Sep 2, 2020

Choose a reason for hiding this comment

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

Doesn't Bech32 imply segwit?

Copy link
Copy Markdown
Contributor Author

@xandronus xandronus Sep 2, 2020

Choose a reason for hiding this comment

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

Maybe should clarify as p2sh-segwit addresses are not supported. Not sure we need to worry about them as bech32 seems to have been well adopted

if (!addressType.Equals("legacy", StringComparison.InvariantCultureIgnoreCase) && !addressType.Equals("bech32", StringComparison.InvariantCultureIgnoreCase))
throw new RPCServerException(RPCErrorCode.RPC_METHOD_NOT_FOUND, "Only address type 'legacy' and 'bech32' are currently supported.");
}

WalletAccountReference accountReference = this.GetWalletAccountReference();

HdAddress hdAddress = this.walletManager.GetUnusedAddresses(accountReference, 1, alwaysnew: true).Single();

string base58Address = hdAddress.Address;
string address = hdAddress.Address; // legacy address
if (addressType != null && addressType.Equals("bech32", StringComparison.InvariantCultureIgnoreCase))
address = hdAddress.Bech32Address;

return new NewAddressModel(base58Address);
return new NewAddressModel(address);
}

/// <summary>
/// RPC method that gets the last unused address for receiving payments.
/// Uses the first wallet and account.
/// </summary>
/// <param name="account">Parameter is deprecated.</param>
/// <param name="addressType">Address type, currently only 'legacy' is supported.</param>
/// <param name="addressType">Address type, currently only 'legacy' and 'bech32' are supported.</param>
/// <returns>The new address.</returns>
[ActionName("getunusedaddress")]
[ActionDescription("Returns the last unused address for receiving payments.")]
Expand All @@ -207,15 +209,18 @@ public NewAddressModel GetUnusedAddress(string account, string addressType)
throw new RPCServerException(RPCErrorCode.RPC_METHOD_DEPRECATED, "Use of 'account' parameter has been deprecated");

if (!string.IsNullOrEmpty(addressType))
{
// Currently segwit and bech32 addresses are not supported.
if (!addressType.Equals("legacy", StringComparison.InvariantCultureIgnoreCase))
throw new RPCServerException(RPCErrorCode.RPC_METHOD_NOT_FOUND, "Only address type 'legacy' is currently supported.");
{
// Currently segwit addresses are not supported.
if (!addressType.Equals("legacy", StringComparison.InvariantCultureIgnoreCase) && !addressType.Equals("bech32", StringComparison.InvariantCultureIgnoreCase))
throw new RPCServerException(RPCErrorCode.RPC_METHOD_NOT_FOUND, "Only address type 'legacy' and 'bech32' are currently supported.");
}
HdAddress hdAddress = this.walletManager.GetUnusedAddress(this.GetWalletAccountReference());
string base58Address = hdAddress.Address;

return new NewAddressModel(base58Address);
string address = hdAddress.Address; // legacy address
if (addressType != null && addressType.Equals("bech32", StringComparison.InvariantCultureIgnoreCase))
address = hdAddress.Bech32Address;
Copy link
Copy Markdown
Member

@sondreb sondreb Sep 2, 2020

Choose a reason for hiding this comment

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

This could be simplified to avoid performing two get operations every time the user request bech32 address. The assignment should depend on the conditional check.


return new NewAddressModel(address);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Tests/Blockcore.Features.RPC.Tests/Controller/WalletRPCControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void GetNewAddress_WithIncompatibleAddressType_ThrowsException()
});

Assert.NotNull(exception);
Assert.Equal("Only address type 'legacy' is currently supported.", exception.Message);
Assert.Equal("Only address type 'legacy' and 'bech32' are currently supported.", exception.Message);
}

[Fact]
Expand All @@ -113,7 +113,7 @@ public void GetUnusedAddress_WithIncompatibleAddressType_ThrowsException()
});

Assert.NotNull(exception);
Assert.Equal("Only address type 'legacy' is currently supported.", exception.Message);
Assert.Equal("Only address type 'legacy' and 'bech32' are currently supported.", exception.Message);
}
}
}