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

Eco-credit Module Proof of Concept Init #125

Merged
merged 13 commits into from
Nov 5, 2020
75 changes: 75 additions & 0 deletions proto/regen/ecocredit/v1alpha1/events.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
syntax = "proto3";

package regen.ecocredit.v1alpha1;

option go_package = "github.com/regen-network/regen-ledger/x/ecocredit";

// EventCreateClass is an event emitted when a credit class is created.
message EventCreateClass {

// class_id is the unique ID of credit class.
string class_id = 1;

// designer is the designer of the credit class.
string designer = 2;
aaronc marked this conversation as resolved.
Show resolved Hide resolved
}

// EventCreateBatch is an event emitted when a credit batch is created.
message EventCreateBatch {

// class_id is the unique ID of credit class.
string class_id = 1;
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason that these identifiers (class_id) are string and not integer?

I guess for batch_denom its useful to have as a string, as that's the same type as denoms in the broader cosmos sdk, but for class_id how do you see these ID's being generated? In my head, this would have been an integer identifier, indexed (as in a relational database) and auto-incrementing

Copy link
Member Author

Choose a reason for hiding this comment

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

An integer converted to a string and so that it's obviously part of denom. We can reassess later


// batch_denom is the unique ID of credit batch.
string batch_denom = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you be more precise here: is the batch_denom unique inside the given class, or globally?

Also, why we are calling it batch_denom ? Why not batch_id? I know that we use denom for an asset name, but here it's meaning (without a comment) is not very clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep I can add some clarifying language on denominations


// issuer is the issuer of the credit batch.
string issuer = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will be an address, right? Maybe let's put that in a comment:

// credit batch issuer account address

Copy link
Member Author

Choose a reason for hiding this comment

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

yep

aaronc marked this conversation as resolved.
Show resolved Hide resolved

// total_units is the total number of units in the credit batch.
string total_units = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we use units (here and below)? I think the term amount is coined almost everywhere where we talk about tokens.

Copy link
Member Author

Choose a reason for hiding this comment

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

for now I'm trying to stay consistent with the RFC that uses the term units and went through a review process. I'd rather this be addressed in the RFC PR #107

}

// EventReceive is an event emitted when credits are received either upon
// creation of a new batch or upon transfer. Each batch_denom created or
// transferred will result in a separate EventReceive for easy indexing.
message EventReceive {

// from indicates the originator of the credits - either an issuer (in the case of Msg/CreateBatch) or a
// sender (in the case of Msg/Send).
oneof from {
// issuer is the issuer of the credit batch in the case that this event is the result of a new batch being created.
// It will not be set if credits were transferred.
string issuer = 1;

// sender is the sender of the credits in the case that this event is the result of a transfer.
// It will not be set if credits were issued.
string sender = 2;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is only one issuer, so we don't need to code it here. In Ethereum, the standard is to put sender=0 (zero value) for minting / issuance transfers. Maybe we can use the same thing here (sender = "")?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Other idea, instead of oneof would be to add a boolean flag here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have to use oneof true, but they are mutually exclusive. I can just make sender = "" if that's preferred

Copy link
Collaborator

Choose a reason for hiding this comment

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

The whole thing is to eliminate theoneof and normalize the data in the RDBMS sense (so to not use the issuer field at all here). Adding a boolean flag is only cosmetic if we want to be explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

well i'm trying to distinguish between issuer and sender as roles...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know. My point is that issuer is known and it must be the same as the batch issuer. By adding it here we denormalize the data.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, so I'm removing and just leaving sender = ""


// recipient is the recipient of the credits
string recipient = 3;

// batch_denom is the unique ID of credit batch.
string batch_denom = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, so I think this answers my comment above: batch_denom is unique globally.

Copy link
Member Author

Choose a reason for hiding this comment

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

yep


// units is the decimal number of both tradable and retired credits received.
string units = 5;
}

// EventRetire is an event emitted when credits are retired. An separate event is emitted
// for each batch_denom in the case where credits from multiple batches have been retired at once
// for easy indexing.
message EventRetire {

// retirer is the account which has done the "retiring". This will be the account receiving credits in
// the case that credits were retired upon issuance using Msg/CreateBatch or retired upon transfer
// using Msg/Send.
string retirer = 1;

// batch_denom is the unique ID of credit batch.
string batch_denom = 2;

// units is the decimal number of credits that have been retired.
string units = 3;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should also add a Claim Subject or Claim Reference - which is used to link the credits retire events with real world event.

Copy link
Member Author

Choose a reason for hiding this comment

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

ditto, this should be part of RFC process

107 changes: 107 additions & 0 deletions proto/regen/ecocredit/v1alpha1/query.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
syntax = "proto3";

package regen.ecocredit.v1alpha1;

import "regen/ecocredit/v1alpha1/types.proto";

option go_package = "github.com/regen-network/regen-ledger/x/ecocredit";

// Msg is the regen.ecocredit.v1alpha1 Query service.
service Query {
// ClassInfo queries for information on a credit class.
rpc ClassInfo(QueryClassInfoRequest) returns (QueryClassInfoResponse);

// BatchInfo queries for information on a credit batch.
rpc BatchInfo(QueryBatchInfoRequest) returns (QueryBatchInfoResponse);

// Balance queries the balance (both tradable and retired) of a given credit batch for a given account.
rpc Balance(QueryBalanceRequest) returns (QueryBalanceResponse);

// Supply queries the tradable and retired supply of a credit batch.
rpc Supply(QuerySupplyRequest) returns (QuerySupplyResponse);

// Precision queries the number of decimal places that can be used to represent credit batch units.
// See Tx/SetPrecision for more details.
rpc Precision(QueryPrecisionRequest) returns (QueryPrecisionResponse);
}

// QueryClassInfoRequest is the Query/ClassInfo request type.
message QueryClassInfoRequest {

// class_id is the unique ID of credit class to query.
string class_id = 1;
}

// QueryClassInfoResponse is the Query/ClassInfo request type.
message QueryClassInfoResponse {

// info is the ClassInfo for the credit class.
ClassInfo info = 1;
}

// QueryBatchInfoRequest is the Query/BatchInfo request type.
message QueryBatchInfoRequest {

// batch_denom is the unique ID of credit batch to query.
string batch_denom = 1;
}

// QueryBatchInfoResponse is the Query/BatchInfo response type.
message QueryBatchInfoResponse {

// info is the BatchInfo for the credit batch.
BatchInfo info = 1;
}

// QueryBalanceRequest is the Query/Balance request type.
message QueryBalanceRequest {

// account is the address of the account whose balance is being queried.
string account = 1;

// batch_denom is the unique ID of credit batch balance to query.
string batch_denom = 2;
}

// QueryBalanceResponse is the Query/Balance response type.
message QueryBalanceResponse {

// tradable_units is the decimal number of tradable units.
string tradable_units = 1;

// retired_units is the decimal number of retired units.
string retired_units = 2;
}

// QuerySupplyRequest is the Query/Supply request type.
message QuerySupplyRequest {

// batch_denom is the unique ID of credit batch to query.
string batch_denom = 1;
}

// QuerySupplyResponse is the Query/Supply response type.
message QuerySupplyResponse {

// tradable_units is the decimal number of tradable units in the batch supply.
string tradable_supply = 1;

// retired_supply is the decimal number of retired units in the batch supply.
string retired_supply = 2;
}


// QueryPrecisionRequest is the Query/Precision request type.
message QueryPrecisionRequest {

// batch_denom is the unique ID of credit batch to query.
string batch_denom = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't all assets from the same asset class have the same precision? If yes, then it's better to have a compact info about the asset and precision ClassInfo instead of having this method.

Copy link
Member Author

Choose a reason for hiding this comment

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

not necessarily, they correspond to different projects which have wildly different sizes

}

// QueryPrecisionResponse is the Query/Precision response type.
message QueryPrecisionResponse {

// max_decimal_places is the maximum number of decimal places that can be used to represent some quantity of credit units.
// It is an experimental feature to concretely explore an idea proposed in https://github.com/cosmos/cosmos-sdk/issues/7113.
uint32 max_decimal_places = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two things to consider:

  1. Using natural numbers
  2. Using fixed decimals

Ad 1. Natural numbers are very fast and they works. There are very good libraries for doing 128-bit arithmetic. 128 bit max value is 3.4e38. And even if we will have 1e9 denomination (like in bitcoin), we could easily multiply numbers up to 1e19 (1e10 denominated in 1e9) without doing any conversion. Otherwise we can easily convert to big.Int for more complex operations.

Ad 2. Having fixed decimals works very well with natural numbers and tooling. All tools will know that they need to put a comma at a certain position, without querying an asset. We could still limit the tradeable amounts by defining granularity - this is what ERC777 is doing (which is also implemented by OpenZeppelin).

Copy link
Member Author

Choose a reason for hiding this comment

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

Under the hood this is basically big.Int. It simply allows configurable granularity so that doesn't need to be decided up front. It would be good to have these discussions in the context of cosmos/cosmos-sdk#7113. This is simply intended to show a viable PoC pathway towards that

}
167 changes: 167 additions & 0 deletions proto/regen/ecocredit/v1alpha1/tx.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
syntax = "proto3";

package regen.ecocredit.v1alpha1;

option go_package = "github.com/regen-network/regen-ledger/x/ecocredit";

// Msg is the regen.ecocredit.v1alpha1 Msg service.
service Msg {

// CreateClass creates a new credit class with an approved list of issuers and optional metadata.
rpc CreateClass(MsgCreateClassRequest) returns (MsgCreateClassResponse);

// CreateBatch creates a new batch of credits for an existing credit class. This will create a new batch denom
// with a fixed supply. Issued credits can be distributed to recipients in either tradable or retired form.
rpc CreateBatch(MsgCreateBatchRequest) returns (MsgCreateBatchResponse);

// Send sends tradeable credits from one account to another account. Sent credits can either be tradable or retired on receipt.
rpc Send(MsgSendRequest) returns (MsgSendResponse);

// Retire retires a specified number of credits in the holder's account.
rpc Retire(MsgRetireRequest) returns (MsgRetireResponse);

// SetPrecision allows an issuer to increase the decimal precision of a credit batch. It is an experimental feature
// to concretely explore an idea proposed in https://github.com/cosmos/cosmos-sdk/issues/7113. The number of decimal
// places allowed for a credit batch is determined by the original number of decimal places used with calling CreatBatch.
// SetPrecision allows the number of allowed decimal places to be increased, effectively making the supply more
// granular without actually changing any balances. It allows asset issuers to be able to issue an asset without needing
// to think about how many subdivisions are needed upfront. While it may not be relevant for credits which likely have
// a fairly stable market value, I wanted to experiment a bit and this serves as a proof of concept for a broader
// bank redesign where say for instance a coin like the ATOM or XRN could be issued in its own units rather than
// micro or nano-units. Instead an operation like SetPrecision would allow trading in micro, nano or pico in the future
// based on market demand. Arbitrary, unbounded precision is not desirable because this can lead to spam attacks (like
// sending 0.000000000000000000000000000001 coins). This is effectively fixed precision so under the hood it is still
// basically an integer, but the fixed precision can be increased so its more adaptable long term than just an integer.
rpc SetPrecision(MsgSetPrecisionRequest) returns (MsgSetPrecisionResponse);
}

// MsgCreateClassRequest is the Msg/CreateClass request type.
message MsgCreateClassRequest {

// designer is the address of the account which designed the credit class. The designer has special permissions
// to change the list of issuers and perform other administrative operations.
string designer = 1;

// issuers are the addresses of the approved issuers.
aaronc marked this conversation as resolved.
Show resolved Hide resolved
repeated string issuers = 2;

// metadata is any arbitrary metadata to attached to the credit class.
bytes metadata = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think that there will be an url string in all classes metedata? If some, maybe it's worth to lift it up?

Copy link
Member Author

Choose a reason for hiding this comment

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

likely, but here I'm trying to postpone dealing with data formats and just deal with asset mechanics. I'm assuming people will put some json here so this basically is the decision to start schema-less and iterate

}

// MsgCreateClassResponse is the Msg/CreateClass response type.
message MsgCreateClassResponse {

// class_id is the unique ID of the newly created credit class.
string class_id = 1;
}

// MsgCreateBatchRequest is the Msg/CreateBatch request type.
message MsgCreateBatchRequest {

// issuer is the address of the batch issuer.
string issuer = 1;

// class_id is the unique ID of the class.
string class_id = 2;

// issuance are the credits issued in the batch.
repeated BatchIssuance issuance = 3;

// metadata is any arbitrary metadata to attached to the credit batch.
bytes metadata = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is one very important field in all issuance requests: a certificate. I would prefer to have it in the top scope, rather hidden in metadata. Certificate is basically a string with url or a proof. Probably we should even formalize it - and have additional service which will be able to prove the certificates.

At minimum, I would add a comment with TODO to consider it.

Copy link
Member Author

Choose a reason for hiding this comment

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

let's open a separate issue to discuss metadata. it's a big topic that I'd rather specify off-chain first and then put on-chain. we would need to discuss with Ron on the customer side


// BatchIssuance represents the issuance of some credits in a batch to a single recipient.
message BatchIssuance {

// recipient is the account of the recipient.
string recipient = 1;

// tradable_units are the units of credits in this issuance that can be traded by this recipient.
// Decimal values are acceptable.
string tradable_units = 2;

// retired_units are the units of credits in this issuance that are effectively retired by the issuer on receipt.
// Decimal values are acceptable.
string retired_units = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why issuing retired credits? We can keep it for flexibility, but I'm curious if this makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, shouldn't we use oneof for tradeable and retired units / credits?

Copy link
Member Author

@aaronc aaronc Nov 4, 2020

Choose a reason for hiding this comment

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

It's a fundamental part of the use case. It's not mutually exclusive either. You could issue both. In some cases it is desirable or even required to issue credits as retired (i.e. non-tradable) on creation. This is more a business question, happy to share more context at some point

}
}

// MsgCreateBatchResponse is the Msg/CreateBatch response type.
message MsgCreateBatchResponse {

// batch_denom is the unique ID of the newly created batch.
string batch_denom = 1;
}

// MsgSendRequest is the Msg/Send request type.
message MsgSendRequest {

// sender is the address of the account sending credits.
string sender = 1;

// sender is the address of the account receiving credits.
string recipient = 2;

// credits are the credits being sent.
repeated SendUnits credits = 3;

// SendUnits are the tradable and retired units of a credit batch to send.
message SendUnits {

// batch_denom is the unique ID of the credit batch.
string batch_denom = 1;

// tradable_units are the units of credits in this issuance that can be traded by this recipient.
// Decimal values are acceptable within the precision returned by Query/Precision.
string tradeable_units = 2;

// retired_units are the units of credits in this issuance that are effectively retired by the issuer on receipt.
// Decimal values are acceptable within the precision returned by Query/Precision.
string retired_units = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above, retired units are not tradeable - so doesn't it make sense to send it over?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, but likely yes. A broker may specifically sell credits that can't be traded as part of the contract. Again a business discussion to have with Ron

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, there are cases where sold credits should be "auto-retired".

Copy link
Member

Choose a reason for hiding this comment

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

Seems strange to allow sending of pre-retired credits. IMO retiring should be a separate action done intentionally by the holder of credits.

We already cover the "retire on issuance" use case in the BatchIssuance message, so why is this needed here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessarily. I believe a broker may specifically only sell credits that are retired on sale. We need to discuss with Ron It may or may not be a use case but better to be on the safe side

Copy link
Member

Choose a reason for hiding this comment

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

agreed cf. #125 (comment)

}
}

// MsgSendResponse is the Msg/Send response type.
message MsgSendResponse { }

// MsgRetireRequest is the Msg/Retire request type.
message MsgRetireRequest {

// holder is the address of the holder of the credits.
aaronc marked this conversation as resolved.
Show resolved Hide resolved
string holder = 1;

// credits are the credits being retired.
repeated RetireUnits credits = 2;

// RetireUnits are the units of the batch being retired.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this description should use the same language as the comment in line 134. Also, related to the comment above about changing a vocabulary for units (in proto/regen/ecocredit/v1alpha1/events.proto)

message RetireUnits {

// batch_denom is the unique ID of the credit batch.
string batch_denom = 1;

// retired_units are the units of credits being retired.
// Decimal values are acceptable within the precision returned by Query/Precision.
string units = 2;
}
}

// MsgRetireRequest is the Msg/Retire response type.
message MsgRetireResponse { }

// MsgRetireRequest is the Msg/SetPrecision request type.
message MsgSetPrecisionRequest {

// issuer is the address of the batch issuer.
string issuer = 1;

// batch_denom is the unique ID of the credit batch.
string batch_denom = 2;

// max_decimal_places is the new maximum number of decimal places that can be used to represent some quantity of
// credit units. It is an experimental feature to concretely explore an idea proposed in https://github.com/cosmos/cosmos-sdk/issues/7113.
uint32 max_decimal_places = 3;
}

// MsgRetireRequest is the Msg/SetPrecision response type.
message MsgSetPrecisionResponse { }
40 changes: 40 additions & 0 deletions proto/regen/ecocredit/v1alpha1/types.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
syntax = "proto3";

package regen.ecocredit.v1alpha1;

option go_package = "github.com/regen-network/regen-ledger/x/ecocredit";

// ClassInfo represents the high-level on-chain information for a credit class.
message ClassInfo {

// class_id is the unique ID of credit class.
string class_id = 1;

// designer is the designer of the credit class.
string designer = 2;

// issuers are the approved issuers of the credit class.
repeated string issuers = 3;

// metadata is any arbitrary metadata to attached to the credit class.
bytes metadata = 4;
}

// BatchInfo represents the high-level on-chain information for a credit batch.
message BatchInfo {

// class_id is the unique ID of credit class.
string class_id = 1;

// batch_denom is the unique ID of credit batch.
string batch_denom = 2;

// issuer is the issuer of the credit batch.
string issuer = 3;

// total_units is the total number of units in the credit batch and is immutable.
string total_units = 4;

// metadata is any arbitrary metadata to attached to the credit batch.
bytes metadata = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

related to the comment above - let's consider adding a certificate field.

Copy link
Member Author

Choose a reason for hiding this comment

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

again let's discuss metadata elsewhere. we don't have context on the engineering team to make these sorts of decisions

}
Loading