-
Notifications
You must be signed in to change notification settings - Fork 276
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
feat!: introduce block header into every smart contract execution context #5151
feat!: introduce block header into every smart contract execution context #5151
Conversation
9c8f8eb
to
3437e43
Compare
3437e43
to
3599869
Compare
7c464b8
to
2daddf4
Compare
2daddf4
to
0b133cb
Compare
…text Signed-off-by: Marin Veršić <[email protected]>
0b133cb
to
ce502ac
Compare
fn get_smart_contract_context(state: &state::SmartContract) -> payloads::SmartContractContext { | ||
payloads::SmartContractContext { | ||
authority: state.authority.clone(), | ||
curr_block: state.state.0.curr_block, |
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.
- smart contract and trigger execution (as latest current block header)
There seems to be an error on documentation side:
Since the execution phase of a smartcontract (wasm as a transaction executable) is different from that of a trigger, each pair (block height, what executions refer the block header) is as follows:
>>>>>>>>>> | block committed | >>>>>>>>>> | triggers executed | |
---|---|---|---|---|
current: StateTransaction.curr_block |
k-1 or k, transaction | k, trigger | ||
latest committed: CommonState.specific_state.curr_block |
k-1, query | k, query? |
So a smartcontract refers the current (validating) block header, not the latest one. On the other hand, it is true that a block header that a trigger refers is current and latest committed
EDIT:
I'm not sure about queries during a trigger execution. At least during a by-call trigger execution, it seems ExecuteTransaction
state blindly lets a query pass. So far this issue hasn't surfaced because even if it were connected to visit_query
, that would validate nothing
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.
You are correct. But this is only because our trigger execution model is broken
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, when transaction and trigger execution are integrated and sequenced, such a discussion would be unnecessary
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 this should be fixed for queries that are executing as part of a transaction (like from inside a trigger). They should receive current block header instead of latest block header. Do you agree?
Any other issues to 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.
opened #5171
closes #4958
Block header is provided as context of execution for:
As a consequence of this change block header variable was added to
StateBlock
andStateTransaction
. While doing this I get the feeling thatStateBlock
could take ownership of the whole block that it is validating