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.

Sign up for GitHub

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

PegNet OPR Chain #40

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

PegNet OPR Chain #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66281b4
parse pegnet opr chain
WhoSoup Sep 26, 2019
6d65886
using go mod of branch
WhoSoup Sep 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 82 additions & 9 deletions db/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,16 @@ import (
"github.com/Factom-Asset-Tokens/fatd/db/entries"
"github.com/Factom-Asset-Tokens/fatd/db/metadata"
"github.com/Factom-Asset-Tokens/fatd/db/nftokens"
"github.com/Factom-Asset-Tokens/fatd/db/pegnet"
"github.com/Factom-Asset-Tokens/fatd/fat"
"github.com/Factom-Asset-Tokens/fatd/fat/fat0"
"github.com/Factom-Asset-Tokens/fatd/fat/fat1"
"github.com/pegnet/pegnet/modules/grader"
)

type applyFunc func(*Chain, int64, factom.Entry) (txErr, err error)
// pointer to chain, the entry itself, the eblock, and the position of entry in eblock's entrylist
// if the entry was applied on its own, position and total are -1
type applyFunc func(*Chain, int64, factom.Entry, factom.EBlock, int) (txErr, err error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than trying to adapt the existing db.Chain type, I would prefer to see two Chain types, one for fat and one for pegnet. To the degree that they share some basic member variables, like the Conn and Pool and possibly the Head factom.EBlock, they could share a common type that they both embed. Then the engine needs to be modified to handle both chain types, either with interfaces or simply more bespoke processing functions if necessary.

Doing this will improve the separation of concern for sections of the code.

I actually prefer having duplicate code than tightly coupled code. It is easier to de-duplicate code than de-couple it.


func (chain *Chain) Apply(dbKeyMR *factom.Bytes32, eb factom.EBlock) (err error) {
// Ensure entire EBlock is applied atomically.
Expand All @@ -57,20 +61,20 @@ func (chain *Chain) Apply(dbKeyMR *factom.Bytes32, eb factom.EBlock) (err error)
}

// Insert each entry and attempt to apply it...
for _, e := range eb.Entries {
if _, err = chain.ApplyEntry(e); err != nil {
for i, e := range eb.Entries {
if _, err = chain.ApplyEntry(e, eb, i); err != nil {
return
}
}
return
}

func (chain *Chain) ApplyEntry(e factom.Entry) (txErr, err error) {
func (chain *Chain) ApplyEntry(e factom.Entry, eb factom.EBlock, pos int) (txErr, err error) {
ei, err := entries.Insert(chain.Conn, e, chain.Head.Sequence)
if err != nil {
return
}
return chain.apply(chain, ei, e)
return chain.apply(chain, ei, e, eb, pos)
}

var alwaysRollbackErr = fmt.Errorf("always rollback")
Expand Down Expand Up @@ -110,8 +114,10 @@ func (chain *Chain) applyIssuance(ei int64, e factom.Entry) (issueErr, err error
}

func (chain *Chain) setApplyFunc() {
if !chain.Issuance.IsPopulated() {
chain.apply = func(chain *Chain, ei int64, e factom.Entry) (
if !chain.Identity.IsPopulated() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a reliable way to check if a chain is a pegnet chain. FAT chains can be tracked but not have a populated Identity yet. It would be better to set some flag, like the Chain.Type when the chain is first put into the ChainMap.

chain.Type = fat.TypeFAT2
} else if !chain.Issuance.IsPopulated() {
chain.apply = func(chain *Chain, ei int64, e factom.Entry, eb factom.EBlock, pos int) (
txErr, err error) {
txErr, err = chain.applyIssuance(ei, e)
return
Expand All @@ -121,17 +127,23 @@ func (chain *Chain) setApplyFunc() {
// Adapt to match ApplyFunc.
switch chain.Type {
case fat0.Type:
chain.apply = func(chain *Chain, ei int64, e factom.Entry) (
chain.apply = func(chain *Chain, ei int64, e factom.Entry, eb factom.EBlock, pos int) (
txErr, err error) {
_, txErr, err = chain.ApplyFAT0Tx(ei, e)
return
}
case fat1.Type:
chain.apply = func(chain *Chain, ei int64, e factom.Entry) (
chain.apply = func(chain *Chain, ei int64, e factom.Entry, eb factom.EBlock, pos int) (
txErr, err error) {
_, txErr, err = chain.ApplyFAT1Tx(ei, e)
return
}
case fat.TypeFAT2:
chain.apply = func(chain *Chain, ei int64, e factom.Entry, eb factom.EBlock, pos int) (
txErr, err error) {
err = chain.ApplyFAT2OPR(ei, e, eb, pos)
return
}
default:
panic("invalid FAT type")
}
Expand Down Expand Up @@ -234,6 +246,67 @@ func (chain *Chain) ApplyFAT0Tx(ei int64, e factom.Entry) (tx *fat0.Transaction,
return
}

var tmp grader.BlockGrader
Copy link
Collaborator

Choose a reason for hiding this comment

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

this global is very very suspicious to me. Why can this not be created on the stack as its needed by functions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This global seems more important than to be named tmp which tells me nothing about it.

Copy link
Contributor Author

@WhoSoup WhoSoup Oct 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

this global is very very suspicious to me. Why can this not be created on the stack as its needed by functions?

it can be made a chain struct variable once there is a dedicated pegnet chain but that would surpass its scope.

the variable only needs to exist during the call of (db)chain.Apply() and would ideally be a local variable in there once the pegnet and fat chains are separated


func (chain *Chain) ApplyFAT2OPR(ei int64, e factom.Entry, eb factom.EBlock, pos int) (err error) {
if !eb.IsPopulated() || pos < 0 { // 'processing' entry
Copy link
Collaborator

Choose a reason for hiding this comment

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

This really seems like it should return an error.

Also I believe it is a reasonable expectation that the caller has populated the EBlock. I think it would be best to not check if the eblock is populated and just let panics happen below here as this represents a serious integrity error with the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both "pending" entries and regular entries call the Apply function, but in the case of pending entries the entryblock passed into the function is nil and the position is -1 (unknown):

fatd/engine/process.go

Line 151 in 6d65886

if _, err := chain.Pending.Chain.ApplyEntry(e, factom.EBlock{}, -1); err != nil {

it's not an error because at the time it is called, the entry cannot be validated. it can only be validated in the context of an eblock

return
}

// beginning of every EBlock
if pos == 0 {
grader.InitLX()
ver := uint8(1)
if eb.Height >= 210330 {
ver = uint8(2)
}

prev, err := pegnet.GetGrade(chain.Conn, eb.Sequence-1)
if err != nil {
return err
}

tmp, err = grader.NewGrader(ver, int32(eb.Height), prev)
if err != nil {
return err
}
}

// Every Entry
var extids [][]byte
for _, x := range e.ExtIDs {
extids = append(extids, []byte(x))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this is necessary for compatibility with the BlockGrader.AddOPR method. I'd rather see AddOPR be updated to accept []factom.Bytes, but for now this is ok.

However if you are going to do this, properly make extids to the correct size: len(e.ExtIDs), and then set the values instead of using append.

Also please follow the golang convention of using camel case everywhere and always capitalize both letters of ID, and use a descriptive variable name in the for loop instead of x.

extIDs := make([][]byte, len(e.ExtIDs)
for i, extID := range e.ExtIDs {
    extIDs[i] = []byte(extID)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see this is necessary for compatibility with the BlockGrader.AddOPR method. I'd rather see AddOPR be updated to accept []factom.Bytes, but for now this is ok.

the grading module was written to be library independent

}

tmp.AddOPR(e.Hash[:], extids, []byte(e.Content))
Copy link
Collaborator

Choose a reason for hiding this comment

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

later, if you'd let me, I'd like to review this code as well. At the minimum I may be able to suggest a number of things to improve memory usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

later, if you'd let me, I'd like to review this code as well. At the minimum I may be able to suggest a number of things to improve memory usage.

sure but memory usage hasn't really been a problem. all the objects generated by the grader are ephemeral. they're discarded after a block is graded and only the relevant data is persisted


//fmt.Println(pos, len(eb.Entries))

// After every EBlock
if pos == len(eb.Entries)-1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this pos variable is concerning to me. This variable seems to be important, but it has a very terse name. I'd like a more descriptive name for it at least. Also it feels a lot like something that should be considered part of the state. As state, I think it should probably be a member variable of the future pegnet chain type. This may also be a way to keep the Apply function signatures succinct while still allowing state data to be passed through without the engine having to deal with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also it feels a lot like something that should be considered part of the state.

"pos" is for position, which is the position of the entry being applied inside the eblock. it is the i counter variable of this function:

fatd/db/apply.go

Lines 63 to 68 in 6d65886

// Insert each entry and attempt to apply it...
for i, e := range eb.Entries {
if _, err = chain.ApplyEntry(e, eb, i); err != nil {
return
}
}

graded := tmp.Grade()
winners := graded.WinnersShortHashes()
//fmt.Println("winners", winners)
err := pegnet.InsertGrade(chain.Conn, eb, winners)
if err != nil {
return err
}

oprs := graded.Winners()
if len(oprs) > 0 {
for _, t := range oprs[0].OPR.GetOrderedAssetsUint() {
err = pegnet.InsertRate(chain.Conn, eb, t.Name, t.Value)
if err != nil {
return err
}
}
}
tmp = nil
graded = nil
}
return
}

func (chain *Chain) ApplyFAT1Tx(ei int64, e factom.Entry) (tx *fat1.Transaction,
txErr, err error) {
tx = fat1.NewTransaction(e)
Expand Down
91 changes: 91 additions & 0 deletions db/pegnet/grade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// MIT License
//
// Copyright 2018 Canonical Ledgers, LLC
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to
// deal in the Software without restriction, including without limitation the
// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
// sell copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.

// Package nftokens provides functions and SQL framents for working with the
// "nf_tokens" table, which stores fat.NFToken with owner, creation id, and
// metadata.

package pegnet

import (
"encoding/json"
"fmt"

"crawshaw.io/sqlite"
"github.com/Factom-Asset-Tokens/factom"
)

const CreateTableGrade = `CREATE TABLE "pn_grade" (
"eb_seq" INTEGER NOT NULL,
"winners" BLOB,

UNIQUE("eb_seq")

FOREIGN KEY("eb_seq") REFERENCES "eblocks"
);
`

func InsertGrade(conn *sqlite.Conn, eb factom.EBlock, winners []string) error {
data, err := json.Marshal(winners)
if err != nil {
return err
}

stmt := conn.Prep(`INSERT INTO "pn_grade"
("eb_seq", "winners") VALUES (?, ?);`)
stmt.BindInt64(1, int64(eb.Sequence))
stmt.BindBytes(2, data)
if _, err := stmt.Step(); err != nil {
if sqlite.ErrCode(err) == sqlite.SQLITE_CONSTRAINT_UNIQUE {
return fmt.Errorf("Grade{%d} already exists", eb.Sequence)
}
return err
}

return nil
}

func GetGrade(conn *sqlite.Conn, seq uint32) ([]string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to see the naming convention of SelectGrade be used here. An established convention in the factom package is that functions named Get... make network calls.

stmt := conn.Prep(`SELECT "winners" FROM "pn_grade" WHERE "eb_seq" = ?;`)
defer stmt.Reset()
stmt.BindInt64(1, int64(seq))
hasRow, err := stmt.Step()
if err != nil {
return nil, err
}
if !hasRow {
return nil, nil
}

buf := make([]byte, 2048)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is possible to get the exact size of the buffer using Stmt.ColumnLen:

$ go doc sqlite.Stmt.ColumnLen
package sqlite // import "crawshaw.io/sqlite"

func (stmt *Stmt) ColumnLen(col int) int
    ColumnLen returns the number of bytes in a query result.

    Column indicies start at 0. https://www.sqlite.org/c3ref/column_blob.html

read := stmt.ColumnBytes(0, buf)
if read < 1 {
return nil, nil
}
var winners []string
err = json.Unmarshal(buf[:read], &winners)
if err != nil {
return nil, err
}

return winners, nil
}
61 changes: 61 additions & 0 deletions db/pegnet/rate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// MIT License
//
// Copyright 2018 Canonical Ledgers, LLC
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to
// deal in the Software without restriction, including without limitation the
// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
// sell copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.

// Package nftokens provides functions and SQL framents for working with the
// "nf_tokens" table, which stores fat.NFToken with owner, creation id, and
// metadata.

package pegnet

import (
"fmt"

"crawshaw.io/sqlite"
"github.com/Factom-Asset-Tokens/factom"
)

const CreateTableRate = `CREATE TABLE "pn_rate" (
"eb_seq" INTEGER NOT NULL,
"token" TEXT,
"value" INTEGER,

UNIQUE("eb_seq", "token")

FOREIGN KEY("eb_seq") REFERENCES "eblocks"
);
`

func InsertRate(conn *sqlite.Conn, eb factom.EBlock, token string, value uint64) error {
stmt := conn.Prep(`INSERT INTO "pn_rate"
("eb_seq", "token", "value") VALUES (?, ?, ?);`)
stmt.BindInt64(1, int64(eb.Sequence))
stmt.BindText(2, token)
stmt.BindInt64(3, int64(value))
if _, err := stmt.Step(); err != nil {
if sqlite.ErrCode(err) == sqlite.SQLITE_CONSTRAINT_UNIQUE {
return fmt.Errorf("Rate{%d-%s} already exists", eb.Sequence, token)
}
return err
}

return nil
}
5 changes: 4 additions & 1 deletion db/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/Factom-Asset-Tokens/fatd/db/entries"
"github.com/Factom-Asset-Tokens/fatd/db/metadata"
"github.com/Factom-Asset-Tokens/fatd/db/nftokens"
"github.com/Factom-Asset-Tokens/fatd/db/pegnet"
)

const (
Expand All @@ -43,7 +44,9 @@ const (
addresses.CreateTableTransactions +
nftokens.CreateTable +
nftokens.CreateTableTransactions +
metadata.CreateTable
metadata.CreateTable +
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine for now but we will eventually want separate schemas. But for now it can wait.

pegnet.CreateTableGrade +
pegnet.CreateTableRate
)

// validateOrApplySchema compares schema with the database connected to by
Expand Down
5 changes: 4 additions & 1 deletion engine/chainmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (
"math"
"sync"

"github.com/Factom-Asset-Tokens/fatd/db"
"github.com/Factom-Asset-Tokens/factom"
"github.com/Factom-Asset-Tokens/fatd/db"
"github.com/Factom-Asset-Tokens/fatd/flag"
)

Expand Down Expand Up @@ -133,6 +133,9 @@ func loadChains() (syncHeight uint32, err error) {
}
}()

oprChain := factom.NewBytes32FromString("a642a8674f46696cc47fdb6b65f9c87b2a19c5ea8123b3d2f0c13b6f33a9d5ef")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are hard coding this chain ID into the engine, which I think is the right approach in general, why do we ever need to modify things like fat.ValidTokenNameIDs as that is only used on new unknown chains? Is there ever a place that fatd needs to scan for pegnet chains?

flag.Whitelist = append(flag.Whitelist, *oprChain)

// Set whitelisted chains to Tracked.
for _, chainID := range flag.Whitelist {
Chains.m[chainID] = Chain{ChainStatus: ChainStatusTracked}
Expand Down
2 changes: 1 addition & 1 deletion engine/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func ProcessPending(es ...factom.Entry) error {
// the current time for now. This is the time we first saw the pending
// entry.
e.Timestamp = time.Now()
if _, err := chain.Pending.Chain.ApplyEntry(e); err != nil {
if _, err := chain.Pending.Chain.ApplyEntry(e, factom.EBlock{}, -1); err != nil {
return err
}
chain.Pending.Entries[*e.Hash] = e // Cache the entry in memory.
Expand Down
13 changes: 13 additions & 0 deletions fat/chainid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ import (
// ValidTokenNameIDs returns true if the nameIDs match the pattern for a valid
// token chain.
func ValidTokenNameIDs(nameIDs []factom.Bytes) bool {
if IsPegNetOPR(nameIDs) {
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 create two separate sets of functions. Ones for validating FAT token chains, and ones for validating pegnet chains. that will greatly improve the clarity of the code. The engine and db packages can stitch these things together as needed.

return true
}
if len(nameIDs) == 4 && len(nameIDs[1]) > 0 &&
string(nameIDs[0]) == "token" && string(nameIDs[2]) == "issuer" &&
factom.ValidIdentityChainID(nameIDs[3]) &&
Expand All @@ -40,6 +43,11 @@ func ValidTokenNameIDs(nameIDs []factom.Bytes) bool {
return false
}

func IsPegNetOPR(nameIDs []factom.Bytes) bool {
return len(nameIDs) == 3 && string(nameIDs[0]) == "PegNet" &&
len(nameIDs[1]) > 0 && string(nameIDs[2]) == "OraclePriceRecords"
}

// NameIDs returns valid NameIDs
func NameIDs(tokenID string, issuerChainID *factom.Bytes32) []factom.Bytes {
return []factom.Bytes{
Expand All @@ -55,6 +63,11 @@ func ChainID(tokenID string, issuerChainID *factom.Bytes32) factom.Bytes32 {

func TokenIssuer(nameIDs []factom.Bytes) (string, *factom.Bytes32) {
var identityChainID factom.Bytes32

if IsPegNetOPR(nameIDs) {
return "", &identityChainID
}

copy(identityChainID[:], nameIDs[3])
return string(nameIDs[1]), &identityChainID
}
3 changes: 3 additions & 0 deletions fat/issuance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ func (i Issuance) MarshalJSON() ([]byte, error) {

// NewIssuance returns an Issuance initialized with the given entry.
func NewIssuance(entry factom.Entry) Issuance {
if IsPegNetOPR(entry.ExtIDs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does pegnet really have an issuance entry at all?

If it does it's probably pretty different from this Issuance type. pegnet should probably just have its own data structures package that implements everything it needs independent of existing FAT types, except where there is a benefit to share.

return Issuance{Type: TypeFAT2, Supply: 1, Entry: Entry{Entry: entry}}
}
return Issuance{Entry: Entry{Entry: entry}}
}

Expand Down
Loading