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

refactor: move Account.get_exitable_utxos/1 into watcher security #1256

Merged
merged 4 commits into from
Jan 14, 2020

Conversation

unnawut
Copy link
Contributor

@unnawut unnawut commented Jan 14, 2020

Overview

This PR moves OMG.WatcherInfo.API.Account.get_exitable_utxos/1 to OMG.Watcher.

Thanks to @jrhite for spotting this boundary breach:

@jrhite: Just ran across this code in WatcherInfo. Seems strange that it's in Info using OMG.DB. Curious why it's in Info, what's the rationale for it? There's a simple query for the same info in db/transaction.ex, but currently it only filters unspent or unexited txoutputs. It could be extended to also filter out txoutputs in an IFE state or other various scenarios.

Thanks!

  @doc """
  Gets all utxos belonging to the given address.
  Slow operation, compatible with security-critical.
  """
  # TODO this seems weird and a breach of decoupling
  @spec get_exitable_utxos(OMG.Crypto.address_t()) :: list(OMG.State.Core.exitable_utxos())
  def get_exitable_utxos(address) do
    # OMG.DB.utxos() takes a while.
    {:ok, utxos} = OMG.DB.utxos()
    OMG.State.Core.standard_exitable_utxos(utxos, address)
  end

Changes

  • Move OMG.WatcherInfo.API.Account.get_exitable_utxos/1 to OMG.Watcher.API.Account
  • Tests

Testing

These Too Should All Pass:

$ mix test test/omg_watcher/api/account_test.exs
$ mix test test/omg_watcher_rpc/web/controllers/account_test.exs

@unnawut unnawut self-assigned this Jan 14, 2020
@unnawut unnawut changed the title Move Account.get_exitable_utxos/1 into watcher security refactor: move Account.get_exitable_utxos/1 into watcher security Jan 14, 2020
@coveralls
Copy link

coveralls commented Jan 14, 2020

Coverage Status

Coverage remained the same at 88.138% when pulling da3b4e6 on unnawut/move-watcher-security-function into 6f4d00f on master.

@unnawut unnawut merged commit 7fb8ffb into master Jan 14, 2020
@unnawut unnawut deleted the unnawut/move-watcher-security-function branch January 14, 2020 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants