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

Time-invariant event log #2382

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 10 additions & 5 deletions byzcoin/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ type CreateGenesisBlock struct {
// the BlockInterval is only used to calculate the maximum protocol
// timeouts and the time-window of acceptance of a new block.
BlockInterval time.Duration
// Maximum block size. Zero (or not present in protobuf) means use the default, 4 megs.
// MaxBlockSize is the maximum block size.
// Zero (or not present in protobuf) means use the default, 4 megs.
// optional
MaxBlockSize int
// DarcContracts is the set of contracts that can be parsed as a DARC.
// DarcContractIDs is the set of contracts that can be parsed as a DARC.
// At least one contract must be given.
DarcContractIDs []string
}
Expand Down Expand Up @@ -173,9 +174,13 @@ type CheckAuthorizationResponse struct {
// ChainConfig stores all the configuration information for one skipchain. It
// will be stored under the key [32]byte{} in the tree.
type ChainConfig struct {
BlockInterval time.Duration
Roster onet.Roster
MaxBlockSize int
// BlockInterval defines the maximum propagation time in the skipchain.
BlockInterval time.Duration
// Roster defines which nodes participate in the skipchain.
Roster onet.Roster
// MaxBlockSize defines the maximum block size on the skipchain.
MaxBlockSize int
// DarcContractIDs is the set of contracts that can be parsed as a DARC.
DarcContractIDs []string
}

Expand Down
3 changes: 1 addition & 2 deletions byzcoin/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1979,8 +1979,7 @@ func (s *Service) startTxPipeline(scID skipchain.SkipBlockID) chan struct{} {
log.Panicf("Fatal error while searching for skipchain %x: %+v\n"+
"DB is in bad state and cannot find skipchain anymore."+
" This function should never be called on a skipchain that does"+
" not exist. DB is in bad state and cannot find skipchain"+
" anymore.",
" not exist.",
scID[:], err)
}

Expand Down
45 changes: 33 additions & 12 deletions eventlog/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ const contractName = "eventlog"
const logCmd = "log"

// Set a relatively low time for bucketMaxAge: during peak message arrival
// this will pretect the buckets from getting too big. During low message
// this will protect the buckets from getting too big. During low message
// arrival (< 1 per 5 sec) it does not create extra buckets, because time
// periods with no events do not need buckets created for them.
const bucketMaxAge = 5 * time.Second

// eventDelayTolerance indicates how much time after the event timestamp
// we are stilling willing to accept an event as genuine and contemporaneous,
// and hence as valid. Expressed in [ns]
const eventDelayTolerance = time.Duration(-60 * 1e9)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const eventDelayTolerance = time.Duration(-60 * 1e9)
const eventDelayTolerance = time.Minute

And reverse the comment to say before, as well as changing the code.


func init() {
var err error
sid, err = onet.RegisterNewService(ServiceName, newService)
Expand Down Expand Up @@ -143,12 +148,11 @@ filter:
return reply, nil
}

func decodeAndCheckEvent(coll byzcoin.ReadOnlyStateTrie, eventBuf []byte) (*Event, error) {
// Check the timestamp of the event: it should never be in the future,
// and it should not be more than 30 seconds in the past. (Why 30 sec
// and not something more auto-scaling like blockInterval * 30?
// Because a # of blocks limit is too fragile when using fast blocks for
// tests.)
func decodeAndCheckEvent(rst byzcoin.ReadOnlyStateTrie, eventBuf []byte) (*Event, error) {
// Check the timestamp of the event: it should never be in the future beyond the current block,
Copy link
Member

Choose a reason for hiding this comment

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

you accept 4 * config.BlockInterval, which is fine, but doesn't match your comment here.

// and it should not be more than 30 seconds in the past before the current block.
// (Why 30 sec and not something more auto-scaling like blockInterval * 30?
// Because a # of blocks limit is too fragile when using fast blocks for tests.)
//
// Also: An event a few seconds into the future is OK because there might be
// time skew between a legitimate event producer and the network. See issue #1331.
Expand All @@ -158,12 +162,29 @@ func decodeAndCheckEvent(coll byzcoin.ReadOnlyStateTrie, eventBuf []byte) (*Even
return nil, err
}
when := time.Unix(0, event.When)
now := time.Now()
if when.Before(now.Add(-30 * time.Second)) {
return nil, fmt.Errorf("event timestamp too long ago - when=%v, now=%v", when, now)

config, err := rst.LoadConfig()
if err != nil {
return nil, err
}

tr, ok := rst.(byzcoin.TimeReader)
if !ok {
return nil, fmt.Errorf("internal error: cannot convert ReadOnlyStateTrie to TimeReader")
}

currentBlockTs := time.Unix(0, tr.GetCurrentBlockTimestamp())

lowerBound := currentBlockTs.Add(eventDelayTolerance)

// Acceptable positive clock skew in the system, mirroring block acceptance criteria.
upperBound := currentBlockTs.Add(4 * config.BlockInterval)

if when.Before(lowerBound) {
return nil, fmt.Errorf("event timestamp too long ago - when=%v, current block=%v", when, currentBlockTs)
}
if when.After(now.Add(5 * time.Second)) {
return nil, errors.New("event timestamp is too far in the future")
if when.After(upperBound) {
return nil, fmt.Errorf("event timestamp is too far in the future - when=%v, current block=%v", when, currentBlockTs)
}
return event, nil
}
Expand Down
Loading