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

History entries does not reflect the real time the an event occured #111

Open
Chengxuan opened this issue Mar 21, 2024 · 3 comments · May be fixed by #117
Open

History entries does not reflect the real time the an event occured #111

Chengxuan opened this issue Mar 21, 2024 · 3 comments · May be fixed by #117
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@Chengxuan
Copy link
Contributor

Chengxuan commented Mar 21, 2024

The timestamp of each event is calculated on AddSubStatus function:

However, the invocation of AddSubStatus function reflects when an event was recorded but not when it occurred.

So the interface will need to change to allow a timestamp to be passed in

@Chengxuan Chengxuan added bug Something isn't working good first issue Good for newcomers labels Mar 21, 2024
@Philip-21
Copy link

Philip-21 commented Apr 1, 2024

Hey @Chengxuan will it be best implementing the Now() in the firefly-common library to explicitly check for specific timestamps

func Now(t ...time.Time) *FFTime {
	var currentTime time.Time
	if len(t) > 0 {
                //extracts current time and coverts to UTC
		currentTime = t[0].UTC()
	} else {
		currentTime = time.Now().UTC()
	}
	tt := FFTime(currentTime)
	return &tt
}

Currently this is the Now() in the firefly-common library

func Now() *FFTime {
	t := FFTime(time.Now().UTC())
	return &t
}

@Chengxuan
Copy link
Contributor Author

Chengxuan commented Apr 2, 2024

@Philip-21 I feel Now should always return the time that Now function is executed instead of a time that is not now.

To address this issue, we need to move away from the now time at this layer as it's already too late, so this layer would need to take in a timestamp.

e.g.

func (p *sqlPersistence) AddSubStatusAction(ctx context.Context, txID string, subStatus apitypes.TxSubStatus, action apitypes.TxAction, info *fftypes.JSONAny, errInfo *fftypes.JSONAny)

becomes

func (p *sqlPersistence) AddSubStatusAction(ctx context.Context, txID string, subStatus apitypes.TxSubStatus, action apitypes.TxAction, info *fftypes.JSONAny, errInfo *fftypes.JSONAny, t *fftypes.FFTime)

@Philip-21
Copy link

alright

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants