Skip to content

Commit

Permalink
Node API - getrawchangeaddress part 1 (PR updates)
Browse files Browse the repository at this point in the history
- On closer inspection, the `reserve_keyinstance` database function should make the read and write step both atomic as they are wrapped in a single `ExecutorItem` on the write thread. Therefore, a `DatabaseUpdateError` should not be expected to ever occur unless the sqlite or python bindings are broken. In which case this is a sign that the wallet is in an undefined / broken state so is not to be included in the Node API documentation for expected errors.
  • Loading branch information
AustEcon authored and rt121212121 committed Mar 20, 2023
1 parent f193198 commit b02b0f0
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 16 deletions.
3 changes: 0 additions & 3 deletions docs/standalone/building-on-electrumsv/node-wallet-api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -849,9 +849,6 @@ call processing are described above.
| A wallet used by the JSON-RPC API must only have one account so that the
API code knows which to make use of. The given wallet has either no accounts
or more than one account (the current number indicated by the `count`).
:Message: | ``Database update error - The key was allocated by something else between the read and the write. Failed to reserve a key``
| This error is only theoretical and would result from a race of the key being
| already allocated by another task.
gettransaction
~~~~~~~~~~~~~~
Expand Down
13 changes: 3 additions & 10 deletions electrumsv/nodeapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
from .transaction import TransactionContext, TransactionFeeEstimator, XTxInput, XTxOutput
from .types import Outpoint
from .util import constant_time_compare
from .wallet_database.exceptions import DatabaseUpdateError


if TYPE_CHECKING:
from .wallet import Wallet
Expand Down Expand Up @@ -919,15 +919,8 @@ async def jsonrpc_getrawchangeaddress_async(request: web.Request, request_id: Re
error=ErrorDict(code=RPCError.WALLET_ERROR,
message=f"Ambiguous account (found {len(accounts)}, expected 1)"))))
account = accounts[0]
try:
key_data = account.reserve_unassigned_key(CHANGE_SUBPATH,
KeyInstanceFlag.IS_RAW_CHANGE_ADDRESS)
except DatabaseUpdateError:
raise web.HTTPInternalServerError(headers={ "Content-Type": "application/json" },
text=json.dumps(ResponseDict(id=request_id, result=None,
error=ErrorDict(code=RPCError.WALLET_ERROR,
message="Database update error - The key was allocated by something else "
"between the read and the write. Failed to reserve a key"))))
key_data = account.reserve_unassigned_key(CHANGE_SUBPATH,
KeyInstanceFlag.IS_RAW_CHANGE_ADDRESS)

# Strictly speaking we return the address of whatever this is. It is almost guaranteed to be
# a base58 encoded P2PKH address that we return.
Expand Down
2 changes: 1 addition & 1 deletion electrumsv/wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -1908,7 +1908,7 @@ def reserve_keyinstance(self, account_id: int, masterkey_id: int,
Returns a future.
The result of the future is the allocated `keyinstance_id` if successful.
Raises `KeyInstanceNotFoundError` if there are no available key instances.
Raises `DatabaseUpdateError` if something else allocated the selected keyinstance first.
Raises `DatabaseUpdateError` should only happen if the sqlite or python bindings are broken.
"""
return db_functions.reserve_keyinstance(self._db_context, account_id,
masterkey_id, derivation_path, allocation_flags)
Expand Down
4 changes: 2 additions & 2 deletions electrumsv/wallet_database/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2058,7 +2058,7 @@ def reserve_keyinstance(db_context: DatabaseContext, account_id: int, masterkey_
Returns the allocated `keyinstance_id` if successful.
Raises `KeyInstanceNotFoundError` if there are no available key instances.
Raises `DatabaseUpdateError` if something else allocated the selected keyinstance first.
Raises `DatabaseUpdateError` should only happen if the sqlite or python bindings are broken.
"""
assert allocation_flags & KeyInstanceFlag.USED == 0
# The derivation path is the relative parent path from the master key.
Expand Down Expand Up @@ -2089,7 +2089,7 @@ def _write(db: Optional[sqlite3.Connection]=None) \
# The result of the read operation just happens to be the parameters we need for the write.
cursor = db.execute(sql_write, (allocation_flags, keyinstance_row[0], KeyInstanceFlag.USED))
if cursor.rowcount != 1:
# The key was allocated by something else between the read and the write.
# This should only happen if the sqlite or python bindings are broken.
raise DatabaseUpdateError()

return cast(int, keyinstance_row[0]), DerivationType(keyinstance_row[1]), \
Expand Down

0 comments on commit b02b0f0

Please sign in to comment.