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

[700 SigmaUSD] Contracts with new parameters #30

Closed
greenhat opened this issue May 19, 2022 · 11 comments · Fixed by #75
Closed

[700 SigmaUSD] Contracts with new parameters #30

greenhat opened this issue May 19, 2022 · 11 comments · Fixed by #75
Labels
bounty help wanted Extra attention is needed

Comments

@greenhat
Copy link
Member

greenhat commented May 19, 2022

After #28

1. Make contract instances static via lazy_static (see existing ORACLE_CONFIG).

  1. Make contract ctor's to build contracts with new parameters(via with_* calls);
  2. Load the contracts(P2S, indices for constant) from the config and use parameters loaded from the config file (token ids, etc) to make updated contracts (via ctor's from 1.)
  3. Ideally, we'd want to check that the contracts with updated parameters from 2. are the same as currently deployed in the pool. This could be achieved by logging a warning when we're fetching the boxes from the node in Scan. We cannot panic since we could be in the middle of updating contracts/boxes via voting.

EDIT: static contract instances are not viable since we need to be able to pass(inject) contract instances in tests without config file.

@greenhat greenhat added this to the v2.0 milestone May 19, 2022
@greenhat greenhat changed the title Contracts with new parameters [200 SigmaUSD] Contracts with new parameters Jun 6, 2022
@greenhat greenhat added help wanted Extra attention is needed bounty labels Jun 6, 2022
@kettlebell
Copy link
Collaborator

I'll do this, since I've indirectly started this issue in #57.

@kettlebell
Copy link
Collaborator

kettlebell commented Jul 19, 2022

@greenhat the line count for this PR keeps climbing. I just got a quick question for ergo box validation. Take the ballot box for example. The UTXO set-scan for this box has the following scanning criteria

let scan_json = json! ( {
"predicate": "and",
"args": [
{
"predicate": "containsAsset",
"assetId": ballot_token_id.clone(),
},
{
"predicate": "equals",
"value": ballot_contract_address.to_base16_bytes().unwrap(),
},
{
"predicate": "equals",
"register": "R4",
"value": ballot_add_bytes.clone(),
}

Now within BallotBoxWrapper::new(ergo_box: ErgoBox, <other args>) I also validate the value ballot token Id, the value in R4 etc. of the box, as these values now reside in the oracle config YAML. Now it's redundant since we already check them in the set-scan above, but I also think it's safer to check them again there. What do you think?

@greenhat
Copy link
Member Author

Oh wow! That's quite a rabbit hole. I'm upping the bounty.

Yes, I think we should check in the constructor for the required properties (regs and whatnot). Today we received the boxes from this scan, but tomorrow that might come from a different source.

@greenhat greenhat changed the title [200 SigmaUSD] Contracts with new parameters [500 SigmaUSD] Contracts with new parameters Jul 19, 2022
@kettlebell
Copy link
Collaborator

kettlebell commented Jul 19, 2022

Thank you! I'll continue adding the checks.

Some more issues to consider:

  • the config YAML will pretty much contain everything, and will serve as ground truth for the app when validating values. I think we need to look into updating the YAML whenever we have a state change, like when operator casts a new vote. Things will get more complicated if we want to keep a history of things that have occurred, too.
  • Should we also bake into the app a formal waiting time to allow a transaction(s) to be confirmed in the chain?

@greenhat
Copy link
Member Author

Some more issues to consider:

* the config YAML will pretty much contain everything, and will serve as ground truth for the app when validating values. I think we need to look into updating the YAML whenever we have a state change, like when operator casts a new vote. Things will get more complicated if we want to keep a history of things that have occurred, too.

I agree that the config file becomes the source of truth. However, in the case of operator voting, it's too early to update the conf file since the actual change in the boxes and contracts will occur after the update box operation. Since it's a very rare operation (voting and updating the pool box/contract), I'd leave it for operators to manually change the conf file. At least for v2.0. We can automate it later if necessary.

* Should we also bake into the app a formal waiting time to allow a transaction(s) to be confirmed in the chain?

I'm not sure I get it right. If in the above context, then it's not necessary.

@kettlebell
Copy link
Collaborator

Got it, thanks!

@kettlebell
Copy link
Collaborator

@greenhat the current PR is large but I hope you're ok with me completing #65 here as well, as it naturally interacts with code in this PR.

@greenhat
Copy link
Member Author

Sure. I believe it could even make the PR smaller. :)
Well, not the PR itself but the code we end up with after the merge.

@kettlebell
Copy link
Collaborator

It will definitely be cleaner code! Maybe lines of code will be similar. I'm opting not to implement serde::Deserialize but rather do

#[serde(
    try_from = "TypeYaml",
    into = "TypeYaml"
)]

where TypeYaml can derive Serialize and Deserialize. It might be a bit more verbose, but I think it's clearer. Implementing Deserialize manually looks really complicated.

@kettlebell
Copy link
Collaborator

Oh yes, I remember first seeing you do this in sigma-rust.

@greenhat greenhat changed the title [500 SigmaUSD] Contracts with new parameters [700 SigmaUSD] Contracts with new parameters Jul 25, 2022
@greenhat
Copy link
Member Author

The bounty is sent. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants