Initial implementation #4
Conversation
abukosek
force-pushed
the
andrej/feature/initial-implementation
branch
4 times, most recently
from
37424c8 to
103eee0
Compare
There was a problem hiding this comment.
Consider using the logging framework from Oasis Core instead of fmt.Printf.
Otherwise the most problematic thing seems to be the transaction identifiers, see my inline comments. Also we should probably consider using Bech32 for account identifiers as discussed and we should use the hash of the canonical genesis document as the network identifier instead of just the chain ID field (as this is what we use internally).
| Index: genBlk.Height, | ||
| Hash: genBlk.Hash, | ||
| }, | ||
| Peers: []*types.Peer{}, // TODO |
There was a problem hiding this comment.
This will require a method in the consensus backend to return the list of peers (or just a similar Status method that returns all these things like the current and genesis block metadata together with the list of peers to avoid multiple requests). Having something like a Status method for the consensus backend seems useful anyway.
abukosek
force-pushed
the
andrej/feature/initial-implementation
branch
8 times, most recently
from
c7404df to
fbe4dcc
Compare
| const OpBurn = "Burn" | ||
|
|
||
| // OpStatusOK is the OK status. | ||
| const OpStatusOK = "OK" |
There was a problem hiding this comment.
One potential issue which may or may not be problematic: this currently only emits successful operations as events are only generated once transactions succeed. Failures will currently never be propagated.
| Index: genBlk.Height, | ||
| Hash: genBlk.Hash, | ||
| }, | ||
| Peers: []*types.Peer{}, // TODO |
abukosek
force-pushed
the
andrej/feature/initial-implementation
branch
2 times, most recently
from
697444a to
7c9f6e2
Compare
abukosek
force-pushed
the
andrej/feature/initial-implementation
branch
2 times, most recently
from
04a504b to
4f4f75c
Compare
abukosek
force-pushed
the
andrej/feature/initial-implementation
branch
from
4f4f75c to
de7486e
Compare
|
OK, I think this is ready for final review now. Thanks for the great comments so far! We can add the remaining stuff in subsequent PRs (using |
Agreed, can you file issues in this repository so that we don't forget about these things? |
Closes #1.