taproot-assets icon indicating copy to clipboard operation
taproot-assets copied to clipboard

Add price oracle proxy

Open sputn1ck opened this issue 11 months ago • 5 comments

This PR adds a price oracle proxy to the rpcserver. The motivation behind is that an application dev might want to reuse the price oracle that is used with the connected tap daemon. Currently this would then mean that the application needs seperate connections for tapd and the price oracle. This simplyfies this.

Closes #https://github.com/lightninglabs/taproot-assets/issues/1356

AI generated summary

This pull request introduces a price oracle proxy feature to the system, along with necessary configurations and tests. The changes include adding new imports, configuration options, and implementing the price oracle proxy functionality in various parts of the codebase.

Key changes include:

Configuration and Imports:

  • Added priceoraclerpc import in multiple files to support the price oracle proxy feature. (config.go, itest/integration_test.go, itest/tapd_harness.go, perms/perms.go, rfq/mock.go, rfq/oracle.go, rpcserver.go, server.go) [1] [2] [3] [4] [5] [6] [7] [8]

Configuration Options:

  • Added AllowPublicPriceOracle to RPCConfig to control public access to the price oracle RPC endpoints. (config.go) [1] [2] [3]
  • Introduced ProxyPriceOracle in Config to proxy price requests to another price oracle. (config.go) [1] [2]

Implementation:

  • Implemented QueryAssetRates method in rpcServer to handle price oracle proxy requests. (rpcserver.go)
  • Registered the PriceOracleServer with the gRPC server. (rpcserver.go)

Testing:

  • Added testPriceOracleProxy to verify the functionality of the price oracle proxy. (itest/integration_test.go)
  • Included the new test in the test cases list. (itest/test_list_on_test.go)
  • Defined a mock implementation for QueryAssetRates in MockPriceOracle. (rfq/mock.go)

Miscellaneous:

  • Added ErrPriceOracleUnimplemented to handle cases where the price oracle service is not implemented. (rpcserver.go)

These changes collectively introduce and test the price oracle proxy feature, ensuring it integrates seamlessly with the existing system.

sputn1ck avatar Feb 18 '25 10:02 sputn1ck

Pull Request Test Coverage Report for Build 13400087732

Details

  • 35 of 48 (72.92%) changed or added relevant lines in 7 files are covered.
  • 61 unchanged lines in 14 files lost coverage.
  • Overall coverage increased (+0.08%) to 54.538%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tapcfg/server.go 3 4 75.0%
perms/perms.go 2 4 50.0%
rfq/oracle.go 0 3 0.0%
rpcserver.go 8 15 53.33%
<!-- Total: 35 48
Files with Coverage Reduction New Missed Lines %
commitment/tap.go 1 85.0%
asset/group_key.go 2 72.65%
fn/iter.go 2 62.07%
mssmt/compacted_tree.go 2 83.27%
tapchannel/aux_leaf_signer.go 2 43.08%
tapdb/mssmt.go 2 88.64%
tapdb/sqlc/mssmt.sql.go 2 52.98%
itest/multisig.go 3 97.86%
rfqmsg/records.go 3 71.2%
universe/base.go 3 79.25%
<!-- Total: 61
Totals Coverage Status
Change from base Build 13397714787: 0.08%
Covered Lines: 48714
Relevant Lines: 89321

💛 - Coveralls

coveralls avatar Feb 18 '25 11:02 coveralls

We could add an RPC endpoint to tapd that returns the host, certificate, and macaroon of the price oracle service. Additionally, we could request a new sub-macaroon from the price oracle to maintain separated permissions.

I think that's a major isolation leak. We should make minimal assumptions about resource or authentication sharing.

Roasbeef avatar Feb 18 '25 18:02 Roasbeef

We could add an RPC endpoint to tapd that returns the host, certificate, and macaroon of the price oracle service. Additionally, we could request a new sub-macaroon from the price oracle to maintain separated permissions.

I think that's a major isolation leak. We should make minimal assumptions about resource or authentication sharing.

I agree and some benefits described in https://github.com/lightninglabs/taproot-assets/issues/1356#issuecomment-2651981505 would not be achieved.

ZZiigguurraatt avatar Jun 20 '25 16:06 ZZiigguurraatt

With that in mind, rather than duplicating the price oracle's RPC endpoint here, we could introduce a new tapd RPC endpoint. Initially, this endpoint would call the price oracle directly. However, in the future, it could leverage the RFQ system to retrieve a more accurate price across edge node peers.

This approach would also factor in asset illiquidity. We could name the new endpoint QueryBestAssetRate, or something similar.

You're basically saying to tie in a bit more with https://github.com/lightninglabs/taproot-assets/issues/1362 ? I think this could be a good idea, but I think we also need to make sure we don't create an unstable system where price oracle queries depend on RFQ and RFQ depends on price oracle queries.

ZZiigguurraatt avatar Jun 20 '25 16:06 ZZiigguurraatt

@sputn1ck, remember to re-request review from reviewers when ready

lightninglabs-deploy avatar Dec 08 '25 05:12 lightninglabs-deploy