Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
v0 Oracle Changes #167
base: master
Are you sure you want to change the base?
v0 Oracle Changes #167
Changes from all commits
44f904c
b369642
df6f714
fe53767
6b1a02b
67376f9
9d2e6b8
0151ab5
bbe3f6b
cba6535
2c7fa3b
dcdc1e3
47b1f49
cb72177
7969fba
e98808b
db54f76
5176f01
6b09bdd
9ea6ce0
9cef39a
aba655b
930f119
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nkohen what is the use for this field? I think it'd be nice to have it explained somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured this would just be nice to have in case some oracle going as
oracle_name
publishes a new public key they use now and now when the user imports the new message, they can see that this is a newer one by looking at the timestamp. Open to changing this, but the idea was to have this be the time at which the oracle first announced this pubkey.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm ok thanks, I'll at least add the explanation and maybe we can discuss it next time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering, what prevents anybody to just releasing a new struct with the name of an existing oracle but changing the public key (and using a newer timestamp)? How do you ensure that it is indeed the correct oracle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is that one way or another people need to get the oracle public key in a good way (e.g. authenticated channel)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nkohen why is there a signature here and then one on the announcement? Isn't it redundant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So as it stands this message is static and can be imported by a wallet independently of an oracle announcement right? In this case it is important that the oracle metadata is what the oracle put down. If the keys are what you expect but someone takes the message and re-writes something about the scheme or description or such, this could be bad for wallet impls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is supposed to be a standalone message, does it make sense to include it in every single announcement? I'll go ahead with that for now but I feel it's also worth discussing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any citation/reason why it degrades security? It seems we state this as truth without some sort of reason why.