-
Notifications
You must be signed in to change notification settings - Fork 621
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
Restore receipts after recent fix #4248
Changes from 116 commits
d7c8fd6
c24c930
f61efed
4e762c7
91d6ef8
3299762
45a9802
8c42e7a
4f8363b
3a7426d
3817c08
d7e903f
af8d2fe
f32b999
4449613
50a8da2
4920661
7a244c9
db2b915
0e5bce2
033da25
a66d7d9
81af983
ab98d81
11e8363
501632c
68a04f9
261de4d
1a9e945
dda95c2
6cc80fb
4257dea
c4cd340
f26d30f
904837d
122c12e
5d00c98
a95e4d0
4861cdf
ee37269
8f9197d
10c6b7a
5d039f3
36ac694
f20ca3a
fdcff6f
4a7309b
d4c0dc4
3a2b50e
dc414bf
cbc0211
907ee3e
771530e
48d8065
ea5b0d0
df7ea6b
d99d161
0de9c82
39bcd10
ae0b5e9
5d7664d
84ffc12
0a4ea03
9cc6ca7
1d68796
2b98daa
acb9d13
13b6471
e90faf7
496c9cb
0a7d2cc
7479882
c3d3e16
518e5f8
39d215a
f3c8f91
3ebed4b
f8aa99e
11b6b02
921d96e
ec1f41c
4c7e883
43511a2
571c082
23e1319
f54b1f8
57cd4f3
d223802
6330623
b857627
edbb264
55d85f0
0edc887
d9877bf
851d1f4
41d7467
a1fef60
32a4a3a
1ccf96c
0c2d79e
2d06f3e
481933b
d8c1e83
109b9a9
921dd04
a925407
32b0116
82e1e18
f7b6e15
1a98485
8bb156b
24a6209
ed93b0d
6a595bf
3062682
faa5065
a60a69e
1f57640
64841ff
4ff65a0
3d530bd
eaf5575
0011ad2
faf5519
b9cfc74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
use crate::store::ChainStoreAccess; | ||
use crate::types::RuntimeAdapter; | ||
use near_chain_primitives::error::Error; | ||
use near_primitives::hash::CryptoHash; | ||
use near_primitives::types::ShardId; | ||
|
||
/// Check that epoch of block with given prev_block_hash is the first one with current protocol version. | ||
fn is_first_epoch_with_protocol_version( | ||
runtime_adapter: &dyn RuntimeAdapter, | ||
prev_block_hash: &CryptoHash, | ||
) -> Result<bool, Error> { | ||
match runtime_adapter.get_prev_epoch_id_from_prev_block(prev_block_hash) { | ||
Ok(prev_epoch_id) => { | ||
let epoch_id = runtime_adapter.get_epoch_id_from_prev_block(prev_block_hash)?; | ||
let protocol_version = runtime_adapter.get_epoch_protocol_version(&epoch_id)?; | ||
let prev_epoch_protocol_version = | ||
runtime_adapter.get_epoch_protocol_version(&prev_epoch_id)?; | ||
Ok(protocol_version != prev_epoch_protocol_version) | ||
} | ||
Err(_) => Ok(true), | ||
} | ||
} | ||
|
||
/// Check that block is the first one with existing chunk for the given shard in the chain with its protocol version. | ||
/// We assume that current block contain the chunk for shard with the given id. | ||
pub fn check_if_block_is_first_with_chunk_of_version( | ||
chain_store: &mut dyn ChainStoreAccess, | ||
runtime_adapter: &dyn RuntimeAdapter, | ||
prev_block_hash: &CryptoHash, | ||
shard_id: ShardId, | ||
) -> Result<bool, Error> { | ||
// At first, check that shard id = 0 and don't do unnecessary computation otherwise | ||
if shard_id != 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure this is still correct way to do this, as in the future we may have migrations that need to be applied on first block with chunk for other shards as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But later we can replace it with the following, right?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure time savings would outweigh additional code complexity. Have you measured how long this runs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It usually takes |
||
return Ok(false); | ||
} | ||
|
||
// Check that block belongs to the first epoch with current protocol version | ||
// to avoid get_epoch_id_of_last_block_with_chunk call in the opposite case | ||
if is_first_epoch_with_protocol_version(runtime_adapter, prev_block_hash)? { | ||
// Compare only epochs because we already know that current epoch is the first one with current protocol version | ||
Ok(chain_store.get_epoch_id_of_last_block_with_chunk(prev_block_hash, shard_id)? | ||
!= runtime_adapter.get_epoch_id_from_prev_block(prev_block_hash)?) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use separate variables to avoid very long expressions like this |
||
} else { | ||
Ok(false) | ||
} | ||
} |
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 looks very suspicious to me. Have you accounted for all possible ways an attacker can maliciously send you some block that such
get_prev_epoch_id_from_prev_block
returns an error?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've tried to handle error coming from
KeyValueRuntime.get_prev_epoch_id_from_prev_block
but now found that my implementation was incorrect. Fixed