Ledger acess rights control #34
Conversation
crypto/bls/mod.go
Outdated
|
|
||
| err := bls.Verify(suite, pk.point, msg, signature.data) | ||
| if err != nil { | ||
| return err |
| return "bls:malformed point" | ||
| } | ||
|
|
||
| return string(buffer)[:4+16] |
There was a problem hiding this comment.
I would comment why those numbers
crypto/bls/mod.go
Outdated
| func (pk publicKey) String() string { | ||
| buffer, err := pk.MarshalText() | ||
| if err != nil { | ||
| return "bls:malformed point" |
There was a problem hiding this comment.
that could help if there were no space, especially in cases where there is some text parsing. Maybe return "bls:malformed_point" ?
|
|
||
| // Register binds a protobuf message type to a public key factory. If a key | ||
| // already exists, it will override it. | ||
| func (f PublicKeyFactory) Register(msg proto.Message, factory crypto.PublicKeyFactory) { |
There was a problem hiding this comment.
why not taking directly a reflect.Type instead of a proto.Message ?
There was a problem hiding this comment.
It seems more natural and user-friendly because you want to register a type of message to be handled by a factory. Also we can switch to something else later if reflect is problematic.
ledger/consumer/smartcontract/mod.go
Outdated
| // Spawn is called to create a new instance. | ||
| Spawn(ctx SpawnContext) (proto.Message, error) | ||
| // Spawn is called to create a new instance. It returns the initial value of | ||
| // the new instance and its access control ID. |
There was a problem hiding this comment.
| // the new instance and its access control ID. | |
| // the new instance and its access rule control (arc) ID. |
ledger/consumer/smartcontract/mod.go
Outdated
| if err != nil { | ||
| return nil, xerrors.Errorf("couldn't read the instance: %v", err) | ||
| } | ||
|
|
||
| contractID := inst.GetContractID() | ||
| contractID := inst.(ContractInstance).GetContractID() |
ledger/consumer/smartcontract/mod.go
Outdated
| func (c Consumer) hasAccess(ctx consumer.Context, key []byte, rule string) error { | ||
| instance, err := ctx.Read(key) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
xerrors.Errorf(...)
same bellow
| hash []byte | ||
| nonce uint64 | ||
| action action | ||
| identity crypto.PublicKey |
There was a problem hiding this comment.
only one identity and one signature?
There was a problem hiding this comment.
Yes because it doesn't make sense to have multiple signatures for one transaction. We can later implement a deferred transaction smart contract to have multiple signatures but when creating a transaction, you usually have one signing key.
ledger/consumer/smartcontract/tx.go
Outdated
| switch tx := txProto.(type) { | ||
| case *SpawnTransactionProto: | ||
| arg, err := f.encoder.UnmarshalDynamicAny(tx.GetArgument()) | ||
| if spawn := txProto.GetSpawn(); spawn != nil { |
There was a problem hiding this comment.
Sorry for being annoying but it's your fault: If the code wasn't that well written I wouldn't care keeping it up to excellence :p
(Broken window theory: even if "as is" it's not that grave I want to avoid any bit of code that could be a sign for one "stranger" to write lower quality code)
crypto/common/mod.go
Outdated
| // of the signature if the message is a known signature message, otherwise it | ||
| // returns an error. | ||
| func (f SignatureFactory) FromProto(in proto.Message) (crypto.Signature, error) { | ||
| if inAny, ok := in.(*any.Any); ok { |
ledger/arc/common/mod.go
Outdated
| // FromProto implements arc.AccessControlFactory. It returns the access control | ||
| // associated with the message. | ||
| func (f *AccessControlFactory) FromProto(pb proto.Message) (arc.AccessControl, error) { | ||
| if pbAny, ok := pb.(*any.Any); ok { |
This adds the ARC principle to Byzcoin with some missing implementation.