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

Remove timestamps from storage and event logs #503

Open
mds1 opened this issue Nov 15, 2021 · 5 comments
Open

Remove timestamps from storage and event logs #503

mds1 opened this issue Nov 15, 2021 · 5 comments
Labels
contracts Smart contract issues

Comments

@mds1
Copy link
Contributor

mds1 commented Nov 15, 2021

This was implemented in #497, but it turns out we don't actually need these helpers because we can get the timestamps from the subgraph + event listeners as needed cc @gdixon @thelostone-mc

@mds1 mds1 added the contracts Smart contract issues label Nov 15, 2021
@metafraction metafraction self-assigned this Nov 16, 2021
@metafraction
Copy link
Member

metafraction commented Nov 16, 2021

Are we effectively reverting this PR or is there stuff we need to keep? most of it seems related to timestamps

@mds1
Copy link
Contributor Author

mds1 commented Nov 16, 2021

Basically reverting it, but we aren't doing this until after GR12 when we flesh out protocol specs/contracts a bit more, because it's already done and the added gas costs on Polygon for GR12 are negligible

@mds1
Copy link
Contributor Author

mds1 commented Nov 19, 2021

@mk1r noticed you were assigned to this, but leaving this unassigned for now since there's currently no plans to work on this

@gdixon
Copy link
Member

gdixon commented Nov 22, 2021

I think we need to keep these changes in place because although in most cases we can use the timestamps provided by the subgraph or the event, when we initially filter the RPC for content between two given blocks (when we don't have a subgraph url), then there is no clear-cut way of retrieving the block/timestamps. Using tx.args.time makes this a lot easier 🙌

@mds1 - how do you feel about closing this out?

@mds1
Copy link
Contributor Author

mds1 commented Nov 23, 2021

then there is no clear-cut way of retrieving the block/timestamps

This is the main thing I'd want to verify before closing this out. When using an event listener with ethers, it's still listening for each block under the hood. (And possibly something for query filters where it already retrieved block info). Meaning that I suspect we might have already queried for block metadata (i.e. timestamp) and it's just discarded/not exposed cleanly in our current approach, but I'm not certain of this

In other words, before closing this out we should verify that there isn't a way to rewrite our event queries to get timestamp for free without an extra RPC call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Smart contract issues
Projects
None yet
Development

No branches or pull requests

3 participants