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

feat: add delegation to sip-007 #1307

Merged
merged 2 commits into from Mar 9, 2020
Merged

feat: add delegation to sip-007 #1307

merged 2 commits into from Mar 9, 2020

Conversation

@kantai
Copy link
Member

kantai commented Mar 2, 2020

Add delegation section to SIP-007

* Delegated Amount: the total amount of Stacks from this address
that the delegate address will be able to issue Stacking messages
on behalf of.
* Reward Address (_optional_): a Bitcoin address that must be

This comment has been minimized.

Copy link
@jcnelson

jcnelson Mar 2, 2020

Member

Do we want to make the description a little more generic by saying "burnchain address"? I can see this technique generalizing to app chains.

This comment has been minimized.

Copy link
@jcnelson

jcnelson Mar 2, 2020

Member

I think this applies to all instances of the word "Bitcoin", if applied.

This comment has been minimized.

Copy link
@kantai

kantai Mar 2, 2020

Author Member

SIP-007 uses "Bitcoin" rather than burnchain in most places, as the SIP is intended to be more specific than the whitepapers which would be more generic.

This comment has been minimized.

Copy link
@jcnelson

jcnelson Mar 3, 2020

Member

Ah, okay. Using "Bitcoin" is fine with me then.

This comment has been minimized.

Copy link
@diwakergupta

diwakergupta Mar 3, 2020

Contributor

Is there a max for the end block?

This comment has been minimized.

Copy link
@kantai

kantai Mar 3, 2020

Author Member

Is there a max for the end block?

I don't think so: if a user wishes to initiate a delegation relationship that lasts for a really long time, that should be allowed as far as the protocol is concerned.

Copy link
Member

jcnelson left a comment

Looks good @kantai ! Just needs some minor word-smithing.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #1307 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1307      +/-   ##
==========================================
+ Coverage   84.74%   84.75%   +<.01%     
==========================================
  Files         161      161              
  Lines       47170    47166       -4     
==========================================
  Hits        39976    39976              
+ Misses       7194     7190       -4
Impacted Files Coverage Δ
src/net/prune.rs 60.46% <0%> (-1.17%) ⬇️
src/util/pipe.rs 86.94% <0%> (-0.75%) ⬇️
src/clarity.rs 58.4% <0%> (-0.37%) ⬇️
src/net/p2p.rs 69.06% <0%> (-0.21%) ⬇️
src/net/http.rs 88.86% <0%> (-0.08%) ⬇️
src/blockstack_cli.rs 82.8% <0%> (-0.07%) ⬇️
src/vm/clarity.rs 84.75% <0%> (-0.06%) ⬇️
src/chainstate/stacks/transaction.rs 92.45% <0%> (-0.06%) ⬇️
src/net/db.rs 79.36% <0%> (-0.05%) ⬇️
src/vm/analysis/type_checker/mod.rs 96.49% <0%> (-0.04%) ⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b9e7c7...c098f5d. Read the comment docs.

Copy link
Member

lgalabru left a comment

Looks good to me, thank you @kantai!

Can we also add a link to this SIP-007 in the readme?

Copy link
Member

jcnelson left a comment

LGTM!

@kantai kantai merged commit 7d16241 into master Mar 9, 2020
3 of 4 checks passed
3 of 4 checks passed
ci/circleci: cargo_tests CircleCI is running your tests
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: test_demo Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.