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

Next deployment todos #28

Closed
2 of 3 tasks
D-Nice opened this issue Dec 15, 2017 · 5 comments
Closed
2 of 3 tasks

Next deployment todos #28

D-Nice opened this issue Dec 15, 2017 · 5 comments
Assignees
Labels

Comments

@D-Nice
Copy link
Contributor

D-Nice commented Dec 15, 2017

  • Make gasprice variable public in connector
  • Make getPrice constant in connector
  • Consider making reqc in connector public at next update, as well as gasprice and other variables. e.g. seeing gasprice would help eth-bridge figure out if it needs to send a tx to change gasprice.
@D-Nice D-Nice added the todo label Dec 15, 2017
@gskapka gskapka self-assigned this Dec 14, 2018
@D-Nice
Copy link
Contributor Author

D-Nice commented May 17, 2019

@gskapka If these are met in the next connector, please checkmark them

@gskapka
Copy link
Collaborator

gskapka commented May 20, 2019

Checkbox 1: Here, are we talking about the addressCustomGasPrice mapping? I'm guessing so since making the uin256 gasPrice = 20e9 public when it's essentially constant is kinda pointless right? So if it is the mapping then no, it's not currently public.

Checkbox 2: Done, now there's loads of overloads too for various usecases.

Checkbox 3: The requestCounter mapping isn't public (yet) either.


So, assuming the above para's assumptions are correct, I've made the necessary changes that'll result in ticking all three check boxes in this issue, and PRd them here.

@gskapka
Copy link
Collaborator

gskapka commented Jun 6, 2019

Update on this:

Checkbox 1: `addressCustomGasPrice is now public & has tests covering that fact.

Checkbox 2: Actually not done, since the oraclizeAPI modifier needs to be on the getPrice fxns for connector-switches, and that modifier changes the state and so fxns using it can't be view. Instead, we discussed showing in the docs how a user can achieve the same thing in their own contract should they wish to.

Checkbox 3: The requestCounter is not public and after vacilliating and making it so then reverting back again, it's been decided to keep it private so as not to foster any external dependencies on it.

@gskapka
Copy link
Collaborator

gskapka commented Jun 7, 2019

And so re this: suggest closing this issue and creating a new one to track the new next-deployment-todos, ie, those we want in v4. v1.4.

(Edited due to following comment)

@D-Nice
Copy link
Contributor Author

D-Nice commented Jun 7, 2019

by v4, you mean v1.4? Sounds good, closing

@D-Nice D-Nice closed this as completed Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants