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
ADR1 #2
ADR1 #2
Changes from all commits
e967050
2c43fa1
fea19dc
698a7fe
eb5329a
b55f982
9bdaa02
4a83fe2
961626a
fb52f06
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.
Shouldn't this also take a block number?
What is
contractId
/ContractId
? An address?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.
Yes and no. Abstractly, yes, but from what I understand of the RPC methods (https://developers.stellar.org/network/soroban-rpc/methods), they themselves only read from the current state, so I'm not sure how you'd implement the RPC calls under the hood if you wanted blocks in the past.
This is from that link you found that I haven't embedded yet (https://developers.stellar.org/docs/smart-contracts/guides/rpc/retrieve-contract-code-python). Quote: "When you deploy a contract, first the code is "installed" (i.e., it is uploaded onto the blockchain). This creates a LedgerEntry containing the Wasm byte-code, which is uniquely identified by its hash (that is, the hash of the uploaded code itself)"
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.
Isn't this a huge limitation for what we want to do?
If you don't want to execute soroban locally, we'd need both the pre and post ledger state of an invocation?
woudl be good to explain this in the ADR
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.
Not necessarily. I imagine an MVP could look like this: You have a monitor with a state property. You read 1 state from the rpc, and verify that this state satisfies the property.
In the next step (v1.1), you tweak the initialization to, instead of reading 1 state, read a sequence of states (which may or may not require reading them one at a time from the rpc and fast-forwarding or rewinding it), and make your TLA spec represent
P(init) /\ P(s1) /\ ... /\ P(sn)
RE the other link, I'll add it in.
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.
This is not unusual. Cosmos and Ethereum also prune the history, but there are archival nodes. We just have to connect to an archival node to get the historical data.
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.
Yeah, actually I don't think you want to use the Soroban RPC, but Horizon instead:
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.
In the beginning, you define State as just a mapping from storage fields to values.
Wouldn't we also need the interactions (a word$w \in {}$ Interface $^+$ ) here?
Or is State !=
State
?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 this comment deserves a more general discussion, because I think the real question being asked here is whether we want a monitor over a trace , or a monitor over a state as the MVP. The idea was, that the critical thing to have is how you turn one state into a set of constraints (which you can then use to generate finite-trace constraints for multiple states).
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.
But to do meaningful monitoring you'd need at least a pair of states / a trace of length 2?
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 don't understand why we are talking about an MVP in the architecture document. I thought we wanted to think through a solid architecture. If we want to go full agile, we don't need many ADRs and RFCs.
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 am still not sure about JSON. Remember, we had to make a few decisions in the ITF format. Actually, why don't we just use the same fragment as in ITF?
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 mean, we can use the same fragment, the point I was trying to make is that instead of designing an IR for State, we write State directly as JSON. The format of that JSON is an orthogonal issue.
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.
Yes, we can write it in json and map it to rust with serde
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 agree with Igor, it's not like State maps 1:1 onto JSON types.
Would be good to record this 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.
+1 for one-to-one name matching.