chore: update borsh and nearcore crate versions in SDK #639
Conversation
austinabell
requested review from
ChaoticTempest,
evgenykuzyakov and
mikedotexe
as code owners
|
buildkite failing because rust version is too low to handle 2021 edition from new nearcore crate releases |
near-sdk/src/test_utils/context.rs
Outdated
| current_account_id: alice().as_str().parse().unwrap(), | ||
| signer_account_id: bob().as_str().parse().unwrap(), | ||
| signer_account_pk: vec![0u8; 32], | ||
| predecessor_account_id: bob().into(), | ||
| predecessor_account_id: bob().as_str().parse().unwrap(), |
There was a problem hiding this comment.
Wouldn't it make more sense to implement two-way Froms for both AccountIds? Something like this:
impl From<near_vm_logic::types::AccountId> for AccountId {
fn from(id: near_vm_logic::types::AccountId) -> Self {
id.into()
}
}
impl From<AccountId> for near_vm_logic::types::AccountId {
fn from(id: AccountId) -> Self {
id.into()
}
}There was a problem hiding this comment.
Because there is no direct conversion between the two. There is no ability to create a vm_logic AccountId without error checking. Can add a TryFrom impl for these, but that didn't seem like it would be worth it, given that it's just used internally and is basically the same amount of code.
| if let Some(index) = receipt_indices.iter().find(|&&el| el >= self.receipts.len() as u64) { | ||
| return Err(HostError::InvalidReceiptIndex { receipt_index: *index }.into()); | ||
| } | ||
| let res = self.receipts.len() as u64; | ||
| self.receipts.push(Receipt { | ||
| receipt_indices, | ||
| receiver_id: AccountId::new_unchecked(receiver_id), | ||
| receiver_id: AccountId::new_unchecked(String::from(receiver_id)), |
There was a problem hiding this comment.
Suggestion above would also help to simplify this (and other changes in this file) to:
| receiver_id: AccountId::new_unchecked(String::from(receiver_id)), | |
| receiver_id: receiver_id.into(), |
There was a problem hiding this comment.
This isn't a public-facing API, no one uses the vm_logic AccountId, and it's generally bad practice to have trait impls for only specific architectures (we cannot have the impl for wasm32-unknown-unknown). I'll consider making some change, though
austinabell
force-pushed
the
austin/nearcore_update
branch
from
c84364f to
b833d9f
Compare
|
commit for suggested change @itegulov |
| ); | ||
| self.receipts | ||
| .get_mut(receipt_index as usize) | ||
| .unwrap() |
There was a problem hiding this comment.
I think we can just change this unwrap to yield an invalid receipt index instead for None
| .unwrap() | |
| .ok_or_else(|| HostError::InvalidReceiptIndex { receipt_index })? |
There was a problem hiding this comment.
Fair point! I didn't change this in the update and this is just used for testing, but this might yield a better error
near-vm-logicandnear-primitives-coreto new version0.10Did not update near-sdk-sim because the dependencies needed were not released and this will be deprecated soon.
There seems to be a lot of new dependencies added for these, which isn't a huge issue but will increase compile times for tests. cc @miraclx if you're curious the diff on the SDK side from the releases :)