search

DeFiGeek-Community / yamato

A keeperless autonomous money maker.
2 stars 1 forks source link

Rinkeby- deploy a mock contract that returns prices from ChainLink/Tellor #12

Closed 0xsomewherecat closed 3 years ago

0xsomewherecat commented 3 years ago

Memo from 0xMotoko: ・RinkebyにChainLink/Tellorの任意数値を定期的に返してくるmock contractを設置(デモにリアル感を持たせるため)……… タスク移譲可 予想3人日

0xsomewherecat commented 3 years ago

@0xMotoko こちら、Definition of Doneの認識合わせをさせてください。

このIssueにおける"mock contract"は、ChainLinkやTellorのふりをして、それっぽい任意数値を返してくるコントラクトをつくってdeployしましょう、ということで合っていますか？

なぜそうするかというと、デモにリアル感をもたせるために数字が定期的に更新されるようにしたいから。（＝Chainlink/TellorのRinkeby contractとつないでもあまり数字が動かないかも、ということ）

実装としては、contracts/PriceFeed.solの中身をつくって、プライス取得用contractをmock contractにする。 ・mock contractのインターフェースはChainlink/Tellorのものに準拠させる。 ・後続フェーズで、プライス取得用contractの向き先をChainlink/Tellorのものにする。

認識ずれているところや、気をつけるべき点などありましたらご教示ください。

0xsomewherecat commented 3 years ago

このIssueはmock contract作成に関わるものなので、PriceFeed.sol実装は別Issueを切った方が良さそうです。 → #16 を起票しました。

0xMotoko commented 3 years ago

Def of Done相違ないです。

もっと言えばPricwFeed.solが読むChainLinkMock.solは初期価格と前回価格をstateとして持ちつつ、価格取得関数では前回価格にランダムにプラスマイナスをかけて返却し、ついでその値を前回価格に上書きするものが良かろうと思います。

0xMotoko commented 3 years ago

取得時に

カオスエンジニアリングとして、

フィード切り替えのテストになる。

0xsomewherecat commented 3 years ago

フィードバックありがとうございます！解像度あがりました。着手します。

0xsomewherecat commented 3 years ago

今週末にv1を完成させることを目標に進めています。

下記のような構成を想定しています。変更した方がよい点がありましたらご教示ください。

  1. Mockコードはyamatoのプロダクションコードではないため、別レポジトリでコミット（名称：priceoracle-mock）

※こちらの点について、yamatoの方にいれた方がよい、などあればご指摘ください。

  1. contracts配下に、二つのモックコントラクトを作成 ChainlinkMock.sol TellorMock.sol

・ChainlinkMockのインターフェースは、latestRoundData()にならう https://docs.chain.link/docs/price-feeds-api-reference/#latestrounddata

・Tellorのインターフェースは、getCurrentValue()にならう https://docs.tellor.io/tellor/integration/introduction

0xMotoko commented 3 years ago

yamatoのcpntracts/mock配下に置いて大丈夫ですよ〜。smockが発明される前まではテスト用コントラクトの量はもっと多かったので、かわいいもんです。

(2)(3)よいと思います

0xsomewherecat commented 3 years ago

ChainlinkMock v1をcontracts/mock配下に置きました。（ライブラリの改良が進んだのですね！）

Motokoさんのフィードバックを元に、下記のような仕様としました。 ・価格を呼ぶ度に、0~1%の範囲で前回価格から変動。（0.1%刻み。変動量・変動の方向+-はランダムに決定） ・1%変化が10回起きると、10回目に前回価格から51%変動。（切替テスト用）

なお、ChainlinkのlatestRoundData()はview関数なのですが、mock関数は内部stateをいじるためviewではありません。

Test caseは、依存性の解決がうまくできておらず試行錯誤中です。

行った手順： ・最新のyamatoコードをpull。ローカルの.envを編集 ・npm install ・npm run compile →正常完了 ・npm run test →下記エラーが発生

FAIL unittest test/unit/CurrencyOS.test.ts ● Test suite failed to run

Cannot find module 'hardhat/internal/hardhat-network/stack-traces/revert-reasons' from 'node_modules/@eth-optimism/smock/dist/src/smockit/binding.js'

Require stack: node_modules/@eth-optimism/smock/dist/src/smockit/binding.js node_modules/@eth-optimism/smock/dist/src/smockit/smockit.js node_modules/@eth-optimism/smock/dist/src/smockit/index.js node_modules/@eth-optimism/smock/dist/src/index.js test/unit/CurrencyOS.test.ts

 at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
 at Object.<anonymous> (node_modules/@eth-optimism/smock/src/smockit/binding.ts:3:1)
0xMotoko commented 3 years ago

package-lock.jsonを削除してから再度npm iするとどうでしょう？

ghost commented 3 years ago

キャッシュ対策としてはpackage-lock.jsonはそのままにして以下を行ってからnpm iを試すのも手です。

hardhatとかtruffleのようなCLIツールはnpmのグローバル領域にインストールしている場合もありますよね。もしそうならそちらも念のためチェックですね。localの方が優先されるはずですが。 この手の問題を予防するためには、CIにてコンテナのようなクリーンな環境下でのビルドチェックを走らせて担保＆壊れたのを検出できるのが一番だと思っています。

0xsomewherecat commented 3 years ago

ありがとうございます！ こちら、MotokoさんにDMでもお付き合い頂き解決しました。

hardhatのバージョンずれと、typechain dirをproject rootに作成する必要がある、という点がポイントでした。

私の場合、hardhatが2.6.0になってしまっていました。TeaTwoさんご指摘の、global領域にインストールしたものが使われるようなコマンドを誤って実行してしまっていた可能性があります。Repoを入れ直し、npm iすることで直りました。

テストケース書きから参加してくれるSolidity初心者などがきた時に有用になると思われますので、yamato README.mdに手順を追記します。

0xMotoko commented 3 years ago

LiquityのPriceFeed.solの実装的にChainlinkMock.sol に decimals() latestRoundData(uint256) が欲しい感じなので、追加おねがいします 🙏

0xsomewherecat commented 3 years ago

承知しました！

decimals()は戻り値のprecisionをチェックするためですね。

latestRoundData(uint256)とお書きになった方は、 getRoundData(uint80 _roundId)の意図と理解しました。（roundIdを指定して過去のプライスを取り寄せる関数で、こちらもLiquityの実装で使われている）

decimals(), getRoundData(uint80 _roundId)を追加します。

0xsomewherecat commented 3 years ago

追加完了しました。

テスト用Mockなので、過去の全ラウンドのデータを保持するよう実装しました。（変な挙動があった場合のdebugに有用…）

Liquity実装が使うのは1つ前のroundDataだけなので、ガス代が気になる環境で動かすことがあればprevious round分だけをもつように変更します。

メモ残課題： ・Chainlink JPY/USD対応。（コンストラクタでシンボル渡して、price初期値とdecimalを制御？） ・Tellor ETH/JPY mock作成。

0xsomewherecat commented 3 years ago

Chainlink Mockについて、コンストラクタで通貨を指定できるようにしました。

RinkebyにデプロイしたMockコントラクト： Chainlink Mock (ETH/USD) 0x81CE5a8399e49dCF8a0ce2c0A0C7015bb1F042bC Chainlink Mock (JPY/USD) 0x6C4e3804ddFE3be631b6DdF232025AC760765279 ※特にみられて困る機能ではないですが、ソースpublish済みのためDiscord project-feedに流れた通知は削除しました。

メモ： ・51%の変動が起きると価格がリアルなものから離れすぎる。こうした場合に、価格を戻したり任意の値をセットできるよう、setPriceToDefault()とsetPrice(price)関数を追加する。

0xsomewherecat commented 3 years ago

メモ： ・setPriceToDefault()とsetPrice(price)関数を追加した。 ・TellorMockの実装が完了。 ・READMEに、Rinkebyの各種Mockコントラクトのアドレスを記録。（※Chainlink Mockは、価格セット関数追加前のversion。QAをした上で、最新版をデプロイする）

ToDo： ・テストケースの実装。

ghost commented 3 years ago

先日のhardhatの依存問題について、今さらながら原因らしきところに気がつきました。

package.jsonのnpmスクリプトでnpxが使われていますが、これはかなりトリッキーです。

package.jsonにhardhatを記述してローカルにインストールしているので、npxを外して"hardhat deploy --network rinkeby"だけで十分です。

npxはターミナルでパッと試したいときに使うお便利ツールで、ローカルに該当パッケージがインストールされてなくてもできちゃうスゴイやつです。逆にいうとローカルのlockファイルは気にしないしnode_modulesに無くても動けちゃいます。これが依存ずれの原因だったのではないかなと疑われますし、今後のためにも外した方が良いと思います。

rollupも同じです。

0xMotoko commented 3 years ago

いろいろ触ってみてのフィードバックを共有させてください！

  1. constructorでmsg.senderをownerにする(あるいはOracleMockBaseがOwnableを継承する)
  2. setLastPrice, setPriceToDefaultをonlyOwnerにする (デモの参加者は幅広いため、不特定多数によるオラクルの改変可能性は実験の停止につながり、本質的ではないけれどもレピュテーションによくない)
  3. chaosの代わりにblock.numberを用いるのが良い気がします。latestRoundDataに不特定多数のユーザーがアクセスするたびに価格が変動するため、おそらく狙ってボラティリティを乱高下できるでしょう。それよりは、不特定多数に変更できないゆっくり更新される整数のほうが適切に思いました。

以上です。

※　EIP1559対応の関係でフォルダ構造をいじっているのと、再度npm iが必要な点、ご留意ください ※2 npxの件、修正中です。ご指摘助かります！ ※3 上記フィードバック、「デモ環境でのMockの安定性確保」としてissue切ってもいいかもしれません

0xsomewherecat commented 3 years ago

フィードバックありがとうございます！

デモ環境で不特定多数の人に叩かれる前提での設計が不足しておりました。具体的なアドバイス大変助かります。

3についてですが、これは、①価格を変動させるトリガーと②51%変動を発生させるトリガーの両方をblock.numberベースのものにした方がよさそう、というサジェスチョンと理解しました。そうすれば、不特定多数のユーザが同時にこの関数を呼んでも値動きはコントロールされたものになる。（内容、同意です！）

こちらの内容、別issue切りますね。→ #27 起票しました。

※1 TeaTwoさん、npxの挙動について勉強になりました！ありがとうございます。 ※2 TellorMockのつくりでお手間をかけてしまった様子ですみません。requireが通らないと、ガス代が暴走するのですね。

0xsomewherecat commented 3 years ago

@0xMotoko かなり時間がかかってしまいましたが、単体テスト実装まで完了しましたのでDoneに移動します。お手数ですが、レビューをお願いできますでしょうか。（ツールと動作環境にはなれることができました）

メモ： ・blocknumberを用いて、1 blockに一度のみ価格を動かせるようにした。（あまりにも急な／高頻度の価格更新を避ける） ・ETH価格の更新にかかわる関数はonlyOwnerとした。

考慮点： ・現状、全通貨ペアにおいて一度の価格更新の変動幅を-1%~1% (0.1%刻み)としている。これは、デモなのである程度大きめに動いてもよしとするか、MainnetのChainlinkのdeviation triggerに合わせた方がよいか（例：ETH/USDであれば-0.5%~0.5%）。 ※Tellorの方は、miningを依頼するuserがTRBでincentiviseする構造になっており、特定の周期・変動幅で必ず価格更新が起きるものではない仕様のため、Chainlinkの変動幅に合わせればよいかと思われます。

0xMotoko commented 3 years ago

問題なさそうです。 node-scheduler などで自動化して動かしながら調整しましょう！ https://github.com/DeFiGeek-Community/yamato/commit/132d71cdf85f7f53df1cf18caff400fa4977ec55

0xsomewherecat commented 3 years ago

早速のご確認ありがとうございます！