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

CSUB-518: Pt. 2 - Set Gate Faucet Address #1276

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Conversation

zacharyfrederick
Copy link
Contributor

@zacharyfrederick zacharyfrederick commented Aug 24, 2023

Rationale

A successful GCRE->CTC swap results in a minting of CTC.

A successful swap of GATE results in a transfer, where the source of funds is an on-chain storage object called the gate faucet address, and the destination is the signer of the extrinsic.

This gate faucet must be set before the request_collect_coins_v2 extrinsic is called. If the on-chain storage value has not been set an error will be raised.

This PR adds a set_gate_faucet extrinsic that allows a key with sudo privileges to specify which on-chain wallet is to be used as the source argument in the transfer function call.

The value gets persisted as an OptionQuery for an AccountId. An OptionQuery will return the None variant when not set.

The getter for the faucet address is Creditcoin::gate_faucet_address. Its definition is the following:

pub type GATEFaucetAddress<T: Config> = StorageValue<_, T::AccountId, OptionQuery>;

Unit tests implemented:
- [x] When not previously set storage value should return the correct option (None)
- [x] The extrinsic can only be set by a root user
- [x] The extrinsic can be set and the storage value is persisted
- [ ] add TODO's here

This PR is based off of Pt. 1. When Pt 1 gets merged into dev this part will get rebased. Will likely need to fix merge conflicts caused by creditcoin-js updates. This pattern will continue for part 3 and 4.

This extrinsic can be called on the javascript side like so:

 await api.tx.sudo
     .sudo(api.tx.creditcoin.setGateFaucet(address))
     .signAndSend(sudoSigner, { nonce: -1 });

@zacharyfrederick zacharyfrederick changed the base branch from dev to csub-518-1-gate-contract August 24, 2023 14:59
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2023

Codecov Report

Merging #1276 (5814f81) into dev (863fad1) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##             dev   #1276   +/-   ##
=====================================
  Coverage   8.32%   8.32%           
=====================================
  Files         28      28           
  Lines        889     889           
  Branches     114     114           
=====================================
  Hits          74      74           
  Misses       815     815           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

For full LLVM coverage report click here!

@atodorov atodorov force-pushed the csub-518-2-gate-faucet-address branch from 05328f2 to 352310f Compare August 25, 2023 11:57
@atodorov atodorov changed the base branch from csub-518-1-gate-contract to dev August 25, 2023 11:58
@atodorov atodorov marked this pull request as draft August 25, 2023 11:58
@atodorov atodorov force-pushed the csub-518-2-gate-faucet-address branch 2 times, most recently from 6be08a2 to 9a3478c Compare August 25, 2023 15:06
@atodorov atodorov marked this pull request as ready for review August 25, 2023 15:06
AdaJane
AdaJane previously approved these changes Aug 25, 2023
Copy link
Contributor

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Working on an integration test for set_gate_contract() and will merge that one first, before merging this one.

Otherwise looks good.

pLabarta
pLabarta previously approved these changes Aug 25, 2023
atodorov
atodorov previously approved these changes Aug 25, 2023
Copy link
Contributor

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

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

A couple of nitpicks but the actual content LGTM

pallets/creditcoin/src/lib.rs Outdated Show resolved Hide resolved
pallets/creditcoin/src/lib.rs Show resolved Hide resolved
pallets/creditcoin/src/lib.rs Show resolved Hide resolved
nathanwhit
nathanwhit previously approved these changes Aug 28, 2023
pLabarta
pLabarta previously approved these changes Aug 28, 2023
Zachary Frederick and others added 3 commits August 29, 2023 11:17
- more descriptive names
- assert on WeightInfo
- return WeightInfo > 0 to pass CI before benchmarks have the chance to
  update the weights with correct values
@atodorov atodorov dismissed stale reviews from nathanwhit and themself via 5814f81 August 29, 2023 08:17
@atodorov atodorov force-pushed the csub-518-2-gate-faucet-address branch from 9374d38 to 5814f81 Compare August 29, 2023 08:17
@atodorov atodorov merged commit 550eb8a into dev Aug 29, 2023
@atodorov atodorov deleted the csub-518-2-gate-faucet-address branch August 29, 2023 10: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.

6 participants