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

♻️ Split seda-chain-contracts into DataRequests and Staking #61

Merged
merged 6 commits into from
Sep 25, 2023

Conversation

jamesondh
Copy link
Contributor

@jamesondh jamesondh commented Sep 14, 2023

Motivation

To facilitate separation of concerns and easier upgradability.

Explanation of Changes

  • Split seda-chain-contracts into DataRequests and Staking
  • Refactored check_eligibility in DataRequests to use a cross-contract query to the proxy to check whether a DR executor is eligible
  • Updated/fixed GH workflows

Testing

Moved some unit tests involving commit/reveal to integration tests, since they require active data request executors

Related PRs and Issues

Closes #56

@jamesondh jamesondh marked this pull request as ready for review September 14, 2023 18:30
@jamesondh jamesondh requested a review from a team September 14, 2023 18:32
@jamesondh jamesondh force-pushed the refactor/split-contract branch from 2ed2451 to c7a23d4 Compare September 20, 2023 19:15
Copy link
Member

@Thomasvdam Thomasvdam left a comment

Choose a reason for hiding this comment

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

I think the focus was mostly on splitting the contracts and that looks fine as far as I can tell.

Comment on lines +33 to +34
/// Posts a data result of a data request with an attached hash of the answer and salt.
/// This removes the data request from the pool and creates a new entry in the data results.
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment is no longer accurate?

Comment on lines +62 to +63
/// Posts a data result of a data request with an attached result.
/// This removes the data request from the pool and creates a new entry in the data results.
Copy link
Member

Choose a reason for hiding this comment

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

Same here, but I guess the focus is on splitting stuff in this PR 😅

@@ -136,7 +137,7 @@ jobs:
devAccount=${{ env.DEV_ACCOUNT }}
if [[ "${{ github.event.inputs.contract }}" != "proxy_contract" ]]; then
proxy=${{ github.event.inputs.proxy }}
OUTPUT=$(./seda-chaind tx wasm instantiate ${{ env.code_id }} '{"token":"aseda","proxy":"${proxy}"}' --no-admin --from ${devAccount} --node ${nodeUrl} --keyring-dir . --keyring-backend test --label ${{ env.code_id }} --gas-prices 0.1aseda --gas auto --gas-adjustment 1.3 -y --output json)
OUTPUT=$(./seda-chaind tx wasm instantiate ${{ env.code_id }} '{"token":"aseda", "proxy": ${{ toJSON(github.event.inputs.proxy) }} }' --no-admin --from ${devAccount} --node ${nodeUrl} --keyring-dir . --keyring-backend test --label ${{ env.code_id }} --gas-prices 0.1aseda --gas auto --gas-adjustment 1.3 -y --output json)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aha nice!

@jamesondh jamesondh merged commit 3415d9c into main Sep 25, 2023
@jamesondh jamesondh deleted the refactor/split-contract branch September 25, 2023 13:47
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.

♻️ Split seda-chain-contracts into DataRequests and Staking
4 participants