Implementing phase 1 spec - add_header #83
Conversation
NIC619
force-pushed
the
implementing_phase_1_spec
branch
from
b1883df
to
2b6b676
Compare
NIC619
force-pushed
the
implementing_phase_1_spec
branch
from
2b6b676
to
106da53
Compare
| ) | ||
| return collation_score | ||
| def get_collation_chunk_root(period: int128, shard_id: int128) -> bytes32: | ||
| return self.collation_records[period][shard_id].chunk_root |
There was a problem hiding this comment.
Why not we just set the collation_records to public, and use its getter to access this state variable? Or does it not work well?
| assert self.receipts[receipt_id].sender == msg.sender | ||
| self.receipts[receipt_id].tx_gasprice = tx_gasprice | ||
| return True | ||
| return True |
sharding/handler/smc_handler.py
Outdated
| chunk_root, | ||
| private_key=None, | ||
| gas=None, | ||
| gas_price=None,): |
| self.shard_head[shard_id] = entire_header_hash | ||
| is_new_head = True | ||
| # Update records_updated_period | ||
| self.records_updated_period[shard_id] = current_period | ||
|
|
There was a problem hiding this comment.
since it's a new header, do we want to clear current_vote[shard_id]'s last byte to 0 here?
There was a problem hiding this comment.
Yes we do! I plan to do this along with submit_vote because then others can get a more clear picture about how current_vote is used when they look into the commit history.
There was a problem hiding this comment.
Got it. Thanks! I'm doing the same implementation in solidity. Feel free to take a look and leave me any feedback : )
prysmaticlabs/prysm#97
| collation_number, | ||
| is_new_head, | ||
| _score, | ||
| chunk_root, |
There was a problem hiding this comment.
If we have self.collation_records[period][shard_id], we don't need addHeader/CollationAdded log now.
There was a problem hiding this comment.
After a second thought, a log can save client some effort as they don't have to keep polling the contract.
|
Decided to move |
NIC619
changed the title
NIC619
changed the title
| # Attempts to process a collation header, returns True on success, reverts on failure. | ||
| @public | ||
| def add_header( | ||
| period: int128, |
There was a problem hiding this comment.
One sudden thought, is it feasible that we remove period from the parameters of add_header?
Since period can only be floor(block.number / self.PERIOD_LENGTH) if add_header success.
There was a problem hiding this comment.
If period was not specified, it is possible that headers intended for different period get included.
There was a problem hiding this comment.
Ah, though the nonce in the transaction might prevent it from being included again, IMO the gas saving is probably not worth it.
There was a problem hiding this comment.
Agree. It is more clear that pass it as parameter.
sharding/handler/smc_handler.py
Outdated
| chunk_root, | ||
| private_key=None, | ||
| gas=None, | ||
| gas_price=None,): |
| # Check that collation record of shard 0 remains the same and transaction consume all gas | ||
| assert smc_handler.records_updated_period(0) == 1 | ||
| assert smc_handler.get_collation_chunk_root(1, 0) == CHUNK_ROOT_1_0 | ||
| assert web3.eth.getTransactionReceipt(tx_hash)['gasUsed'] == default_gas |
There was a problem hiding this comment.
Sorry, second thought: HeaderAdded log that returns True/False may be helpful for the client side to determine whether the add_header tx is included.
There was a problem hiding this comment.
Log will be emitted only if transaction succeed, i.e. passing all the checks. If client catches a HeaderAdded log, he can be sure that header is added.
There was a problem hiding this comment.
Currently I name it AddHeader to conform with log naming pattern, e.g, RegisterNotary, DeregisterNotary.
We can change them to HeaderAdded, NotaryRegistered in the future if we like.
What was wrong?
implement part of SMC modification - stage 1 #76
How was it fixed?
Implement
add_headerandsubmit_votefunctions.add_header:add_headerto submit collation header(i.e.collation_records).records_updated_periodto keep track of in which period was the latest collation record submitted.current_votewhich keeps track of the vote count is cleared once a new collation record is submitted.Cute Animal Picture