feat: Add NervosDAO as a CKB VM script #21

xxuejie · 2019-06-21T03:00:37Z

TODO:

tests

xxuejie · 2019-06-22T01:38:14Z

@doitian I'm running into a recursive dependency problem: in order to add tests for this repo, we need to update the main CKB repo since data structures are changed. However in order to update the main repo, we also need the updated contracts here, and since the main repo can only rely on published crates of ckb-system-scripts, we cannot do that either.

~~So as a result, what about we merge this PR first. I will finish the PR in the main repo, then we can go back and add missing DAO tests in this repo?~~

EDIT: we tried a different route: an RC version is first published to use in the CKB main repo PR, which can solve the problem.

xxuejie · 2019-06-22T02:29:11Z

Tests for NervosDAO are included here

doitian · 2019-06-24T07:48:26Z

c/dao.c

+  unsigned char witness[WITNESS_SIZE];
+
+  len = WITNESS_SIZE;
+  ret = ckb_load_witness(witness, &len, 0, input_index, CKB_SOURCE_INPUT);


🐞Check that len <= WITNESS_SIZE

ℹ️Review Information

Each withdraw consumes a former deposited cell as an input. The corresponding witnesses contain two arguments:

The first is intended to be consumed by the lock script to prove the user owns the cell.

The second one is a u64 encoded in little endian, it is the index into the transaction deps, which references an anchor header. We assume that the cell is withdraw from that anchor header. Transaction submitter should use the most recent block as the anchor header. The interest between anchor header to the block really commit the withdraw transaction is an implicit cost of the withdraw.

c/dao.c

doitian · 2019-06-24T13:34:56Z

Could you please provide a brief description in the PR about:

What is stored in the field dao in the block header.
How a user deposits CKB into DAO and how to withdraw.

doitian · 2019-06-25T01:54:33Z

c/dao.c

+    return ERROR_INVALID_WITHDRAW_BLOCK;
+  }
+
+  unsigned char deposit_dao[DAO_SIZE];


ℹ️Review Information

Reference of the DAO data layout

https://github.com/nervosnetwork/ckb/blob/f1f838a66b37cd433456d04a5831f7d52db19280/util/dao/utils/src/lib.rs#L42

Will add a comment at the top of the script document everything.

c/dao.c

doitian · 2019-06-25T02:20:59Z

c/dao.c

+  unsigned char witness[WITNESS_SIZE];
+
+  len = WITNESS_SIZE;
+  ret = ckb_load_witness(witness, &len, 0, input_index, CKB_SOURCE_INPUT);


ℹ️Review Information

Each withdraw consumes a former deposited cell as an input. The corresponding witnesses contain two arguments:

The first is intended to be consumed by the lock script to prove the user owns the cell.

The second one is a u64 encoded in little endian, it is the index into the transaction deps, which references an anchor header. We assume that the cell is withdraw from that anchor header. Transaction submitter should use the most recent block as the anchor header. The interest between anchor header to the block really commit the withdraw transaction is an implicit cost of the withdraw.

c/dao.c

doitian · 2019-06-25T02:38:17Z

c/dao.c

+  }
+
+  uint64_t counted_capacity = 0;
+  if (__builtin_usubl_overflow(original_capacity, occupied_capacity,


ℹ️Review Information

Occupied capacity does not count when calculating interest.

doitian · 2019-06-25T03:00:54Z

c/dao.c

+        dao_input = 1;
+      }
+    } else if (ret == CKB_ITEM_MISSING) {
+      /* DAO Issuing input here, we can just skip it */


ℹ️Review Information

Issuing input is a special OutPoint which is used as an indicator to tells the transaction verifier that the rule outputs total capacity <= inputs total capacity is handled in this script, dao.c.

https://github.com/nervosnetwork/ckb/blob/d470e5d804f3669d54ed53262a9a002d078a9984/core/src/transaction.rs#L162

doitian

Request changes are marked with 🐞

xxuejie requested review from a team, doitian and janx June 21, 2019 03:00

xxuejie mentioned this pull request Jun 22, 2019

feat: Secondary miner issurance, split DAO as a separate contract nervosnetwork/ckb#1094

Merged

1 task

xxuejie force-pushed the nervos-dao-script branch from 0005da6 to 137d76d Compare June 22, 2019 01:43

janx approved these changes Jun 22, 2019

View reviewed changes

doitian reviewed Jun 24, 2019

View reviewed changes

c/dao.c Show resolved Hide resolved

doitian reviewed Jun 25, 2019

View reviewed changes

c/dao.c Show resolved Hide resolved

doitian reviewed Jun 25, 2019

View reviewed changes

doitian requested changes Jun 25, 2019

View reviewed changes

xxuejie added 9 commits June 27, 2019 06:07

feat: Add NervosDAO as a CKB VM script

08ea478

fix: Update makefile PHONY targets

8732cc1

fix: Calculate the maximum of windowleft and maturity in DAO

4701aca

chore: bump version for testing

c689322

test: NervosDAO tests

7882ee8

fix: Update ckb repo dependency hash

b004c11

fix: Code review fixes

5641542

fix: Tests

9e8255a

chore: Update CKB revision

14f4e3f

xxuejie force-pushed the nervos-dao-script branch from be5317d to 14f4e3f Compare June 27, 2019 06:09

xxuejie requested a review from doitian June 27, 2019 06:09

doitian approved these changes Jun 27, 2019

View reviewed changes

xxuejie merged commit 66d7da8 into master Jun 27, 2019

xxuejie deleted the nervos-dao-script branch June 27, 2019 15:36

doitian mentioned this pull request Jul 4, 2019

Track v0.17.0 changes nervosnetwork/ckb#1178

Closed

1 task

xxuejie mentioned this pull request Jul 26, 2019

feat: upgrade ckb version, extract cell_output data to transaction #26

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add NervosDAO as a CKB VM script #21

feat: Add NervosDAO as a CKB VM script #21

xxuejie commented Jun 21, 2019 •

edited

Loading

xxuejie commented Jun 22, 2019 •

edited

Loading

xxuejie commented Jun 22, 2019

doitian Jun 24, 2019 •

edited

Loading

doitian Jun 25, 2019 •

edited

Loading

doitian commented Jun 24, 2019

doitian Jun 25, 2019 •

edited

Loading

xxuejie Jun 25, 2019

doitian Jun 25, 2019 •

edited

Loading

doitian Jun 25, 2019

doitian Jun 25, 2019 •

edited

Loading

doitian left a comment •

edited

Loading

feat: Add NervosDAO as a CKB VM script #21

feat: Add NervosDAO as a CKB VM script #21

Conversation

xxuejie commented Jun 21, 2019 • edited Loading

xxuejie commented Jun 22, 2019 • edited Loading

xxuejie commented Jun 22, 2019

doitian Jun 24, 2019 • edited Loading

Choose a reason for hiding this comment

doitian Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

doitian commented Jun 24, 2019

doitian Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

xxuejie Jun 25, 2019

Choose a reason for hiding this comment

doitian Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

doitian Jun 25, 2019

Choose a reason for hiding this comment

doitian Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

doitian left a comment • edited Loading

Choose a reason for hiding this comment

xxuejie commented Jun 21, 2019 •

edited

Loading

xxuejie commented Jun 22, 2019 •

edited

Loading

doitian Jun 24, 2019 •

edited

Loading

doitian Jun 25, 2019 •

edited

Loading

doitian Jun 25, 2019 •

edited

Loading

doitian Jun 25, 2019 •

edited

Loading

doitian Jun 25, 2019 •

edited

Loading

doitian left a comment •

edited

Loading