Skip to content

contract instantiation and queries#6

Merged
keyleu merged 5 commits intomasterfrom
keyne/contract-instantiation
Sep 12, 2023
Merged

contract instantiation and queries#6
keyleu merged 5 commits intomasterfrom
keyne/contract-instantiation

Conversation

@keyleu
Copy link
Collaborator

@keyleu keyleu commented Sep 8, 2023

This change is Reviewable

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 10 files reviewed, 5 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)


contract/Cargo.toml line 2 at r1 (raw file):

[package]
name = "xprl-bridge"

Not sure whether it onfluence on it or not, but we agreed to call that bridge coreumbridge-xrpl


contract/src/contract.rs line 36 at r1 (raw file):

) -> CoreumResult<ContractError> {
    set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
    initialize_owner(

How does that func work?


contract/src/contract.rs line 60 at r1 (raw file):

    let xrp_issue_msg = CosmosMsg::from(CoreumMsg::AssetFT(Issue {
        symbol: "xrp".to_string(),

Can we move it xrp to a constant ?


contract/src/msg.rs line 15 at r1 (raw file):

    pub relayers: Vec<Addr>,
    //How many relayers need to sign the message.
    pub threshold: u32,

I would call it evidence_threshold, since we might have signing_threshold later which is a be different.


contract/src/msg.rs line 21 at r1 (raw file):

#[cw_serde]
pub enum Side {

Why do we need that enum?


contract/src/state.rs line 28 at r1 (raw file):

    pub relayers: Vec<Addr>,
    pub threshold: u32,
    pub min_tickets: u32,

IMO the min_tickets if too far to add it now, let's remove it from the current PR.

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 10 files reviewed, 6 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)


contract/src/contract.rs line 36 at r1 (raw file):

) -> CoreumResult<ContractError> {
    set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
    initialize_owner(

Is it possible to query the admin?

Copy link
Collaborator Author

@keyleu keyleu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 10 files reviewed, 6 unresolved discussions (waiting on @dzmitryhil, @miladz68, @wojtek-coreum, and @ysv)


contract/Cargo.toml line 2 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Not sure whether it onfluence on it or not, but we agreed to call that bridge coreumbridge-xrpl

it won't but just renamed it anyways.


contract/src/contract.rs line 36 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

How does that func work?

It's a package will owner functionality, you can initialize an owner, transfer ownership, renounce ownership, query ownership ....


contract/src/contract.rs line 36 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Is it possible to query the admin?

Yes of course.


contract/src/contract.rs line 60 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Can we move it xrp to a constant ?

Done!


contract/src/msg.rs line 15 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

I would call it evidence_threshold, since we might have signing_threshold later which is a be different.

ok done


contract/src/msg.rs line 21 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Why do we need that enum?

It was something I had from previous version, removed it.


contract/src/state.rs line 28 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

IMO the min_tickets if too far to add it now, let's remove it from the current PR.

Ok removed it for now.

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)


contract/src/contract.rs line 36 at r1 (raw file):

Previously, keyleu (Keyne) wrote…

Yes of course.

Is that query (API query) is already available?

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 10 files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

a discussion (no related file):
Approved on my side, but needs one more approval.
@ysv @wojtek-coreum @miladz68


Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 10 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

Copy link
Collaborator Author

@keyleu keyleu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @miladz68, @wojtek-coreum, and @ysv)


contract/src/contract.rs line 36 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Is that query (API query) is already available?

Yes, https://docs.rs/cw-ownable/latest/cw_ownable/

I actually added them to the contract just now because they were not added.

dzmitryhil
dzmitryhil previously approved these changes Sep 12, 2023
Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r4.
Reviewable status: 8 of 10 files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)


contract/.gitignore line 9 at r4 (raw file):

*.iml
\#*\#
.\#*

Add new line here please.

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r3, 1 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

Copy link
Collaborator Author

@keyleu keyleu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @miladz68, @wojtek-coreum, and @ysv)


contract/.gitignore line 9 at r4 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Add new line here please.

Done

Copy link
Contributor

@dzmitryhil dzmitryhil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 10 files at r1, 4 of 6 files at r2, 1 of 2 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @keyleu, @wojtek-coreum, and @ysv)


contract/.gitignore line 9 at r4 (raw file):

*.iml
\#*\#
.\#*

always leave an empty line at the end of files.

Copy link
Collaborator Author

@keyleu keyleu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)


contract/.gitignore line 9 at r4 (raw file):

Previously, miladz68 (milad) wrote…

always leave an empty line at the end of files.

yes, sorry, it was added already in last revision

Copy link
Contributor

@miladz68 miladz68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu, @wojtek-coreum, and @ysv)

@keyleu keyleu merged commit ec1a69d into master Sep 12, 2023
@keyleu keyleu deleted the keyne/contract-instantiation branch January 10, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants