search

omgnetwork / elixir-omg

OMG-Network repository of Watcher and Watcher Info
https://omg.network
Apache License 2.0
211 stars 59 forks source link

Is transaction.submit security critical API? #1129

Closed InoMurko closed 4 years ago

InoMurko commented 4 years ago

The security critical watcher allows you to use /transaction.submit Sends transaction to Child chain. but it does not allow you to retrieve any of the UTXOs or balances.

/account.get_utxos Gets all utxos belonging to the given address.

The question is...

unnawut commented 4 years ago

Or the other way around, keep /transaction.submit as a security critical API, plus bring /account.get_utxos into security critical API. Since both are required to make a transaction. And misinformation/unavailability of these two endpoints could cause inability to interact with the funds?

This would leave the rest of the Informational APIs purely for data retrieval, except for maybe /transaction.submit_typed.

pnowosie commented 4 years ago

@InoMurko - this is correct. Security critical API is not for transacting on the network, but to provide network's security data. So if one wants utxo selection features for transacting it's InfoApi piece of cake. (Security Api provides however all addresses utxos for exit)

/transaction.submit is considered security critical because it checks validity of the chain before it forward the tx to the child chain. (Note: checking validity isn't yet implemented there - so it might confuse you).

InoMurko commented 4 years ago

as we found out with @unnawut 

/account.get_utxos

actually use PG to retrieve utxos per account. So that would be rather non trivial to use on the "security" side of watcher.

Hm... so @pnowosie so one should use informational watcher to retrieve UTXOs and than if he wants to securely transact, he should use /transaction.submit on security watcher? But then your statement is contradicting itself 

Security critical API is not for transacting on the network, but to provide network's security data.

why is /transaction.submit there if API's purpose is to provide network's security data?

unnawut commented 4 years ago

So if one wants utxo selection features for transacting it's InfoApi piece of cake

Note that that's /transaction.create's responsibility though which I agree to have it in informational.

But /transaction.submit can't be done unless it at least has info from /account.get_utxos. Which makes a security API dependent on info API, hence my suggestion to move /account.get_utxos into security.

jrhite commented 4 years ago

account.get_utxos is quite informational too, at least from a block-explorer perspective. Might this be one of the few, if not only, apis where we can allow a little overlap?

InoMurko commented 4 years ago

account.get_utxos is purely PG implemented. So that would be a BIG overlap in terms of code.

pdobacz commented 4 years ago

Ah two things I missed earlier that contribute to this. One was noted by PaweΕ‚ :point_up:

/transaction.submit is considered security critical because it checks validity of the chain before it forward the tx to the child chain

And to chime in on this part:

Security critical API is not for transacting on the network, but to provide network's security data.

Maybe I and PaweΕ‚ we've done a mental shortcut here. I think security critical is necessary but not sufficient to transact on the network.

.

The other comment is that for the security critical Watcher, there is the account.get_exitable_utxos version of the informational-flavored account.get_utxos, for the purpose of getting the utxos in order to exit. But this endpoint should not be used in conjunction with attempts to transact on the child chain, because it can be very slow - it should only be good enough to fire once (when exiting) and be done with it.

I'm bringing account.get_exitable_utxos up only to emphasize, that this is not the endpoint you're looking for here.

I'm not sure yet though, how to move forward here :thinking: .

InoMurko commented 4 years ago

@pdobacz were there ever attempts to store UTXOs per account in RocksDB - as part of Security Critical Watcher?

pdobacz commented 4 years ago

@pdobacz were there ever attempts to store UTXOs per account in RocksDB - as part of Security Critical Watcher?

nope, since the security features did not require that strongly yet.

InoMurko commented 4 years ago

So how would a customer of ours normally have this setup @Pongch @pdobacz ? Lets say you're an institutional client that want's to trade securely using our software.

Would you run the security and informational watcher side by side? And use the UTXO data provided by the informational watcher to transact on the security watcher? If yes, why not use transaction.submit on the informational watcher since the implementation is the same?

unnawut commented 4 years ago

Maybe I and PaweΕ‚ we've done a mental shortcut here. I think security critical is necessary but not sufficient to transact on the network.

  • Security Critical ensures you don't run into security issues when transacting (hence transaction.submit there)
  • Information API actually provides you with fast APIs to efficiently transact (by finding out the balance and utxos)

Does this assume that the two APIs will always belong to the same application and both APIs will always be started and served together?

Because if we want to plan for starting/enabling them separately, we won't be able to do so from a business/user-perspective, because the security API is not really usable without the informational API, and in the most typical use case (making transactions) both security & informational need to be started. So even though we achieve separation of concerns code-wise, we don't achieve a separation of concerns usage-wise in this case.

(Not trying to argue for one approach or another yet but just want to lay out all perspectives)

pnowosie commented 4 years ago

though we achieve separation of concerns code-wise, we don't achieve a separation of concerns usage-wise in this case.

That's correct @unnawut - @pdobacz & I don't argue with it. Unfortunately before the split previous design anticipated that one can have Watcher in these modes:

unnawut commented 4 years ago

@pnowosie Thanks! That's totally understandable.

Now with the split, assuming these are the 3 start modes? These would be their users?

I hope identifying the users would help draw the line. But feel free to flag me if my questions are getting too sidetracked from the topic πŸ˜‚

InoMurko commented 4 years ago

I tihnk this is very valuable! Just to correct you a little bit @unnawut. The codebase at the moment does not allow you

unnawut commented 4 years ago

By the way, the reason I'm asking all this is because I'm helping out on the split, and so I want to get a sense of the ideal architecture going forward, and less so on the things as is. All these info from @pnowosie & @pdobacz and your opinion on the ideal way forward will help me a lot down the line πŸ™‡

unnawut commented 4 years ago

The codebase at the moment does not allow you ...

Yes yes. Sorry those 3 points were more about the ideal case forward (or if they're not, what would be).

pdobacz commented 4 years ago

If yes, why not use transaction.submit on the informational watcher since the implementation is the same?

It might have become clearer already, but to straighten out more: the truly "security related" bit of transaction.submit is not implemented, only planned for. The informational API might not have all the data/means/responsibility to block tx submission when it is not safe to submit.

the security API is not really usable without the informational API

I see that we're deferring to product inputs here (:+1: !) but my understanding so far was:

unnawut commented 4 years ago

Funnelling conversations back here:

I have no further inputs for now. Mainly looking forward to see the personas.

jrhite commented 4 years ago

If I may ask a nΓ€ive question...

If we were starting from scratch, wouldn't it make sense that security API has everything we are discussing + transaction.create/submit? Transaction submission seems very tied to security.

The informational API would be purely for querying.

Are we doing the split the way we are because of previous design decisions that are not easy to undo right now. Or are we doing the split the way we are because that's what we think the logically 'correct' way to do it is?

Pongch commented 4 years ago

Catching up on this conversation now. IMO, defining whether a Watcher API belongs in security critical or informational basket is at the moment a false dichotomy since the purpose of the Watcher is to allow users to make transactions on Plasma in a secured manner. Which is why everything does indeed tie back to security.

From the consumer application side, they are not at the moment opinionated whether an API they are consuming is coming from security or informational, as long as its coming from the same service.

If a split is indeed a service level (1 service for Security, 1 for informational). Then, generally a wallet requirements is for deposit, send and exit operations coming from one service. And that another set of applications would perform challenges or communicate w/ wallet applications to perform other operations ie, getting funds out of the Network in mass exit scenario.

Not sure if this made this discussion more complicated Coming back ~unnawut~ @InoMurko first question:

should we remove transaction.submitfrom the security critical watcher?

Consumer app cares only that the deposit/transact/exit API coming from one service and is agnostic about separation of concerns internally.

pdobacz commented 4 years ago

So, maybe to throw in an alternative idea:

...or (alternative within an alternative) informational/transaction.submit first calls security/transaction.can_submit_now immediately before chch/transaction.submit

In both cases informational remains the only service the "wallet-like" client talks to, on a day-to-day job of deposit/transact/exit

InoMurko commented 4 years ago

Swagger definitions is where I take the truth from. So a correction on:

security drops transaction.submit_typed, renames its transaction.submit to transaction.submit_secure. The latter eventually gets the security features implemented

submit_typed is not part of security watcher.

Info apis: https://developer.omisego.co/elixir-omg/docs-ui/?url=master%2Foperator_api_specs.yaml&urls.primaryName=master%2Finformational_api_specs

Security: https://developer.omisego.co/elixir-omg/docs-ui/?url=master%2Foperator_api_specs.yaml&urls.primaryName=master%2Fsecurity_critical_api_specs

One thing I observed and want to repeat: is that an informational node has security endpoints and informational.

A security node has ONLY security endpoints.

I don’t see a reason why informational endpoint would need to strip out security endpoints.

jrhite commented 4 years ago

The reason I see for informational API dropping all security endpoints is it means zero attack surface for the Informational API.

InoMurko commented 4 years ago

Elaborate pls...

unnawut commented 4 years ago

In response to @pdobacz:

Settled:

security drops transaction.submit_typed

☝️ As Ino mentioned this is already part of informational, so we're good here. πŸŽ‰

informational keeps transaction.create (which belongs there, as it relies heavily on rich queries)

☝️ Yup agree. I think this one is generally agreed upon. πŸŽ‰

Ongoing:

  • security renames its transaction.submit to transaction.submit_secure. The latter eventually gets the security features implemented
  • informational gets transaction.submit and submit_typed
  • informational/transaction.submit and submit_typed only talk to security/transaction.submit_secure underneath (which in turn does chch/transaction.submit)

I was proposing this idea to @InoMurko as well. But the main issue is that while /transaction.submit is in security-critical API/subapp, it requires /account.get_utxos in informational API/subapp to make a transaction, so we can't cleanly split the security vs. info boundary.

And having a cleanly split security vs. info boundary asap means that we can start strong enforcements, e.g. compile time checks complaining any code or tests illegally crossed the border. Where we can then start trusting that any change to informational is really not touching security.

And there're only two ways to enforce strong boundaries for this case:

  1. /transaction.submit and /account.utxos both are in security-critical

  2. /transaction.submit and /account.utxos both are in informational

As far as I understand, option 1 is a huge effort, so option 2 seems more promising. Basically sacrificing by having /transaction.submit in informational (both I and @InoMurko agree it's reallyyyy #^&*%!) in exchange for achieving strict boundaries now.

In medium term, the safe_submit and getting utxos into security can go together...

unnawut commented 4 years ago

This one is sidetracking but kinda related...

I see 3 interpretations of informational being used throughout various conversations:

  1. informational -> deposits/transfers/exits, /transaction.create, basic transaction listing, utxo listing
  2. informational -> deposits/transfers/exits, /transaction.create, basic transaction listing, utxo listing, plus network stats, BI (business intelligence), data aggregations, advanced search and filtering
  3. informational -> network stats, BI (business intelligence), data aggregations, advanced txn search

It's really hard to reach a conclusion because the criticality between 1, 2 and 3 are so far apart.

Can we agree on terms that can differentiate them? How about:

  1. security -> challenges, IFEs
  2. wallet -> deposits/transfers/exits, /transaction.create, basic transaction listing, utxo listing
  3. data -> network stats, BI (business intelligence), data aggregations, advanced search and filtering, /utxo.history, etc.
  4. security + wallet + data -> challenges, IFEs, deposits/transfers/exits, /transaction.create, basic transaction listing, utxo listing, network stats, BI (business intelligence), data aggregations, advanced search and filtering, /utxo.history, etc.
  5. Any other combos of above

Then we know exactly that @Pongch's comment here https://github.com/omisego/elixir-omg/issues/1129#issuecomment-553771714 is actually wallet, while @jrhite's comment here https://github.com/omisego/elixir-omg/issues/1129#issuecomment-553860623 is data. The two interpretations of informational are vastly different.

Edit: Added security in addition to wallet and data.

InoMurko commented 4 years ago

To me, the easiest way to imagine what certain watcher modes mean (and their underlying architecture is what APIs they provide). Less about how they're used - that's harder to limit and imagine what others do with it.

Informational Watcher to me means:

### **_Security related endpoints:_**
/status.get Returns information about the current state of the child chain and the watcher.
/account.get_exitable_utxos Gets all utxos belonging to the given address.
/utxo.get_challenge_data Gets challenge data for a given utxo exit.
/utxo.get_exit_data Gets exit data for a given utxo.
/transaction.submit Sends transaction to Child chain. **QUESTIONABLE**
/in_flight_exit.get_data Gets exit data for an in-flight exit.
/in_flight_exit.get_competitor Returns a competitor to an in-flight exit.
/in_flight_exit.prove_canonical Proves transaction is canonical.
/in_flight_exit.get_input_challenge_data Gets the data to challenge an invalid input piggybacked on an in-flight exit.
/in_flight_exit.get_output_challenge_data Gets the data to challenge an invalid output piggybacked on an in-flight exit.
**_### Plus the existing informational (that are expanded on demand)_**
/transaction.all Gets all transactions (can be limited with various filters).
/transaction.create Finds an optimal way to construct a transaction spending particular amount.
/transaction.get Gets a transaction with the given id.
/transaction.submit_typed Sends EIP-712 formatted transaction to Child chain.
/account.get_balance Returns the balance of each currency for the given account address.
/account.get_utxos Gets all utxos belonging to the given address.
/account.get_transactions Gets a list of transactions for given account address.

And the Security Watcher is really just:

### **_Security related endpoints:_**
/status.get Returns information about the current state of the child chain and the watcher.
/account.get_exitable_utxos Gets all utxos belonging to the given address.
/utxo.get_challenge_data Gets challenge data for a given utxo exit.
/utxo.get_exit_data Gets exit data for a given utxo.
/transaction.submit Sends transaction to Child chain. **QUESTIONABLE**
/in_flight_exit.get_data Gets exit data for an in-flight exit.
/in_flight_exit.get_competitor Returns a competitor to an in-flight exit.
/in_flight_exit.prove_canonical Proves transaction is canonical.
/in_flight_exit.get_input_challenge_data Gets the data to challenge an invalid input piggybacked on an in-flight exit.
/in_flight_exit.get_output_challenge_data Gets the data to challenge an invalid output piggybacked on an in-flight exit.

Why do I see informational watcher having security endpoints? Because of how watcher is designed, we really get these endpoints for free. You cannot disable security properties of watcher, because that IS watcher. The data is producted and stored and by having an API ... you can also read them externally. Everything added on top (as in informational API) is really just aggregating and enriching data that the security properties of watcher already produce.

unnawut commented 4 years ago

I'm not familiar enough with the security API. Can a malicious actor do anything, say if they can access the security APIs through the watcher instance that's publicly serving the block explorer?

unnawut commented 4 years ago

Note that I'm not helping to push the conversation back to "Is transaction.submit security critical API?" here but I think we can close this "informational-only" mode in a few more comments then back to the original question I hope.

unnawut commented 4 years ago

I'm not familiar enough with the security API. Can a malicious actor do anything, say if they can access the security APIs through the watcher instance that's publicly serving the block explorer?

Not really right? Even the security part is all read-only? Maybe except for /transaction.submit which is also not directly writing anything, at most it will be a sanity checker and relayer?

pdobacz commented 4 years ago

I'm not familiar enough with the security API. Can a malicious actor do anything, say if they can access the security APIs through the watcher instance that's publicly serving the block explorer?

Not really right? Even the security part is all read-only?

Current implementation is read-only. transaction.submit, after the security checks on it are added, will not be, and accessing it by an unauthorized actor could have security implications (for example, one could turn those security checks off and always push the transactions, to cover up the byzantine condition).

This is the first thing that comes to my mind, but not entirely sure if this is all if the features of the security Watcher keep evolving, and it becomes even less "read-only".

pnowosie commented 4 years ago

Oh, no, no - I think we have a misunderstanding here. I got @unnawut question the way, he is asking what happens if someone discovers the Watcher address that is providing data to BE and tries to use its security API (consume the API).

If this was the question - the answer is no harm could be done - security critical API is open publicly. (but I can think of DoS vector against BE)

pnowosie commented 4 years ago

I'm not helping to push the conversation back to "Is transaction.submit security critical API?" here but I think we can close this "informational-only" mode in a few more comments

I think @Pongch make it clear: InfoApi should allow for depost, transfer and (standard) exit. It's simple and we should make it happen.

We put some effort to decide in which of these two buckets tx.submit belongs to but it was indeed a "false dichotomy". It does not matter how and when I made my transaction if nobody will challenge an invalid exit.

To summarize it I'd say if I've deposited some funds to plasma chain I either have trust that somebody will keep it secure (so I can go with InfoApi alone) or I will take care of the security myself (and I'm runing Watcher). The approach depends probably more of the amount of the funds than the purity of the separation of concerns. (and it hurts me but πŸ‘ )

I'm not helping either, what do you think @unnawut @pdobacz @InoMurko ❓

unnawut commented 4 years ago

I got @unnawut question the way, he is asking what happens if someone discovers the Watcher address that is providing data to BE and tries to use its security API (consume the API).

Yes that was my original intention. But the answers to both were equally useful though!

unnawut commented 4 years ago

I think what we can close now from @pdobacz's https://github.com/omisego/elixir-omg/issues/1129#issuecomment-553412020, @InoMurko's https://github.com/omisego/elixir-omg/issues/1129#issuecomment-554245907, @Pongch's https://github.com/omisego/elixir-omg/issues/1129#issuecomment-553771714 and @pnowosie's https://github.com/omisego/elixir-omg/issues/1129#issuecomment-556462644 is that....

These two modes continue to exist:

  1. security only
  2. security + informational

And this mode will not be supported:

  1. informational only

Now back to "Is transaction.submit security critical API?"...

unnawut commented 4 years ago

So coming back to /transaction.submit. I think these are the options?

  1. Move /transaction.submit into informational API βœ… Minimal changes βœ… Boundary is respected, boundary checks can be enforced right away πŸ”΄ /transaction.submit security checks coming soon

  2. Keep /transaction.submit in security-critical (call it submit_secure), and add a new /transaction.submit to informational API that forwards to security-critical.

    βœ… Boundary is maintained πŸ”΄ /transaction.submit_secure is still unusable in security-only mode πŸ”΄ The original issue still remains with security + informational needed to transact

  3. Keep /transaction.submit in security-critical. Nothing changes.

    βœ… It is what it is, no changes πŸ”΄ Boundary is loosened, but only in tests?

  4. Move /account.get_utxos into security-critical. βœ… Boundary respected πŸ”΄ Huge effort πŸ”΄ Doesn't belong in security-critical(?)

I lean towards 3 and accept the technical debt, given the cons of other options πŸ€”

unnawut commented 4 years ago

RE: https://github.com/omisego/elixir-omg/pull/1194#pullrequestreview-328874891

2 tests that are breaching the boundary: https://circleci.com/gh/omisego/elixir-omg/30000

  1) test get the blocks from child chain after sending a transaction and start exit (OMG.Watcher.Integration.BlockGetterTest)
     test/omg_watcher/integration/block_getter_test.exs:51
     ** (Postgrex.Error) ERROR 42P01 (undefined_table) relation "txoutputs" does not exist

         query: SELECT t0."blknum", t0."txindex", t0."oindex", t0."owner", t0."amount", t0."currency", t0."proof", t0."spending_tx_oindex", t0."child_chain_utxohash", t0."creating_txhash", t0."spending_txhash", t0."inserted_at", t0."updated_at" FROM "txoutputs" AS t0 LEFT OUTER JOIN "ethevents_txoutputs" AS e2 ON e2."child_chain_utxohash" = t0."child_chain_utxohash" LEFT OUTER JOIN "ethevents" AS e1 ON e2."root_chain_txhash_event" = e1."root_chain_txhash_event" WHERE (((t0."owner" = $1) AND t0."spending_txhash" IS NULL) AND (e1 IS NULL OR 
      NOT EXISTS (SELECT 1
                  FROM ethevents_txoutputs AS etfrag
                  JOIN ethevents AS efrag ON
                           etfrag.root_chain_txhash_event=efrag.root_chain_txhash_event
                           AND efrag.event_type IN ('standard_exit')
                           AND etfrag.child_chain_utxohash = t0."child_chain_utxohash"))) ORDER BY t0."blknum", t0."txindex", t0."oindex"
     code: WatcherHelper.get_utxos(alice.addr)
     stacktrace:
       (ecto_sql) lib/ecto/adapters/sql.ex:618: Ecto.Adapters.SQL.raise_sql_call_error/1
       (ecto_sql) lib/ecto/adapters/sql.ex:551: Ecto.Adapters.SQL.execute/5
       (ecto) lib/ecto/repo/queryable.ex:153: Ecto.Repo.Queryable.execute/4
       (ecto) lib/ecto/repo/queryable.ex:18: Ecto.Repo.Queryable.all/3
       (omg_watcher_rpc) lib/web/controllers/account.ex:38: OMG.WatcherRPC.Web.Controller.Account.get_utxos/2
       (omg_watcher_rpc) lib/web/controllers/account.ex:15: OMG.WatcherRPC.Web.Controller.Account.action/2
       (omg_watcher_rpc) lib/web/controllers/account.ex:15: OMG.WatcherRPC.Web.Controller.Account.phoenix_controller_pipeline/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.instrument/4
       (omg_watcher_rpc) lib/spandex_phoenix.ex:156: OMG.WatcherRPC.Web.Controller.Account.call/2
       (phoenix) lib/phoenix/router.ex:280: Phoenix.Router.__call__/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.plug_builder_call/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint."call (overridable 3)"/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.call/2
       (phoenix) lib/phoenix/test/conn_test.ex:235: Phoenix.ConnTest.dispatch/5
       (omg_watcher) test/support/watcher_helper.ex:64: Support.WatcherHelper.rpc_call/3
       (omg_watcher) test/support/watcher_helper.ex:42: Support.WatcherHelper.success?/2
       test/omg_watcher/integration/block_getter_test.exs:57: (test)
  2) test Thin client scenario (OMG.Watcher.Integration.TransactionSubmitTest)
     test/omg_watcher/integration/transaction_submit_test.exs:48
     ** (Postgrex.Error) ERROR 42P01 (undefined_table) relation "txoutputs" does not exist

         query: SELECT t0."blknum", t0."txindex", t0."oindex", t0."owner", t0."amount", t0."currency", t0."proof", t0."spending_tx_oindex", t0."child_chain_utxohash", t0."creating_txhash", t0."spending_txhash", t0."inserted_at", t0."updated_at" FROM "txoutputs" AS t0 LEFT OUTER JOIN "ethevents_txoutputs" AS e2 ON e2."child_chain_utxohash" = t0."child_chain_utxohash" LEFT OUTER JOIN "ethevents" AS e1 ON e2."root_chain_txhash_event" = e1."root_chain_txhash_event" WHERE (((t0."owner" = $1) AND t0."spending_txhash" IS NULL) AND (e1 IS NULL OR 
      NOT EXISTS (SELECT 1
                  FROM ethevents_txoutputs AS etfrag
                  JOIN ethevents AS efrag ON
                           etfrag.root_chain_txhash_event=efrag.root_chain_txhash_event
                           AND efrag.event_type IN ('standard_exit')
                           AND etfrag.child_chain_utxohash = t0."child_chain_utxohash"))) ORDER BY t0."blknum", t0."txindex", t0."oindex"
     code: } = WatcherHelper.success?("transaction.create", order)
     stacktrace:
       (ecto_sql) lib/ecto/adapters/sql.ex:618: Ecto.Adapters.SQL.raise_sql_call_error/1
       (ecto_sql) lib/ecto/adapters/sql.ex:551: Ecto.Adapters.SQL.execute/5
       (ecto) lib/ecto/repo/queryable.ex:153: Ecto.Repo.Queryable.execute/4
       (ecto) lib/ecto/repo/queryable.ex:18: Ecto.Repo.Queryable.all/3
       (omg_watcher_info) lib/omg_watcher_info/db/txoutput.ex:185: OMG.WatcherInfo.DB.TxOutput.get_sorted_grouped_utxos/1
       (omg_watcher_info) lib/omg_watcher_info/api/transaction.ex:83: OMG.WatcherInfo.API.Transaction.create/1
       (omg_watcher_rpc) lib/web/controllers/transaction.ex:76: OMG.WatcherRPC.Web.Controller.Transaction.create/2
       (omg_watcher_rpc) lib/web/controllers/transaction.ex:15: OMG.WatcherRPC.Web.Controller.Transaction.action/2
       (omg_watcher_rpc) lib/web/controllers/transaction.ex:15: OMG.WatcherRPC.Web.Controller.Transaction.phoenix_controller_pipeline/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.instrument/4
       (omg_watcher_rpc) lib/spandex_phoenix.ex:156: OMG.WatcherRPC.Web.Controller.Transaction.call/2
       (phoenix) lib/phoenix/router.ex:280: Phoenix.Router.__call__/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.plug_builder_call/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint."call (overridable 3)"/2
       (omg_watcher_rpc) lib/web/endpoint.ex:15: OMG.WatcherRPC.Web.Endpoint.call/2
       (phoenix) lib/phoenix/test/conn_test.ex:235: Phoenix.ConnTest.dispatch/5
       (omg_watcher) test/support/watcher_helper.ex:64: Support.WatcherHelper.rpc_call/3
       (omg_watcher) test/support/watcher_helper.ex:42: Support.WatcherHelper.success?/2
       test/omg_watcher/integration/transaction_submit_test.exs:72: (test)
jrhite commented 4 years ago

In terms of what security watcher means, I can't see it making sense for /transaction.submit to live anywhere other than the security watcher. i fully support option 3.

unnawut commented 4 years ago

The info in this conversation staled quite a bit by now. Starting a new ticket to collect similar conversations from different places.