Add AggregatorRouter Contract use case #8
Conversation
package.json
Outdated
| "typescript": "^4.9.4" | ||
| } | ||
| "name": "data-feed-consumer", | ||
| "version": "0.2.0", |
There was a problem hiding this comment.
I think we should also increase version here.
| @@ -7,15 +7,17 @@ import dotenv from 'dotenv' | |||
|
|
|||
| dotenv.config() | |||
|
|
|||
| let commonConfig = { gas: 5_000_000, gasPrice: 250_000_000_000 } | |||
| let commonConfig = {} | |||
There was a problem hiding this comment.
Is there a reason to move definition inside the conditional branch?
There was a problem hiding this comment.
there were error from my environment so I had to change it..
There was a problem hiding this comment.
It complains about accounts, not about gas and gasPrice. Did you try to define accounts as below?
let commonConfig = { gas: 5_000_000, gasPrice: 250_000_000_000, accounts: [] }Then we could just update the accounts
commonConfig.accounts = [process.env.PRIV_KEY]There was a problem hiding this comment.
don't know what's different from my environment but it still leads to error
There was a problem hiding this comment.
You don't have to waste time on it. We can fix it later ;)
contracts/DataFeedConsumer.sol
Outdated
| int256 public answer; | ||
| uint80 public roundId; | ||
|
|
||
| constructor(address aggregatorProxy) { | ||
| constructor(address aggregatorProxy, address aggregatorRouter) { |
There was a problem hiding this comment.
I think passing AggregatorProxy loses its purpose after having access to the AggregatorRouter. How having two consumer contracts? One showing how to use AggregatorProxy and one for AggregatorRouter?
There was a problem hiding this comment.
I felt bothered to make another contract and make separate deploy script, and thought that this is enough explanation for other developers to understand. But I'll write another contract if you insist
There was a problem hiding this comment.
Understood! We should assume that some people might want to take our code and make minimal modifications. In this case, they have to understand both approaches and always remove one of them. Please separate those two approaches into two consumer contracts.
contracts/DataFeedConsumer.sol
Outdated
| , /* uint startedAt */ | ||
| , /* uint updatedAt */ | ||
| , /* uint80 answeredInRound */ | ||
| int256 answer_ /* uint startedAt */ /* uint updatedAt */ /* uint80 answeredInRound */, |
There was a problem hiding this comment.
Can we keep the previous format? 🙏
There was a problem hiding this comment.
it's automatically formatted from my vs code... I'll turn off format on save option and update it
No description provided.