Networking AccountId tracking#1058
Merged
ilblackdragon merged 24 commits intostagingfrom Jul 27, 2019
Merged
Conversation
+ Communication between client and network on trigger
Missing trigger condition
+ Routing table as mechanism to support conection through hops
| .send(NetworkClientMessages::GetChainInfo) | ||
| .into_actor(self) | ||
| .then(move |res, act, _ctx| match res { | ||
| ctx.wait(self.client_addr.send(NetworkClientMessages::GetChainInfo).into_actor(self).then( |
Member
Author
There was a problem hiding this comment.
This against the formatter will. I think we all should use same version of rustfmt.
| #[derive(PartialEq, Eq, Clone, Debug)] | ||
| pub struct AnnounceAccount { | ||
| /// AccountId to be announced | ||
| pub account_id: AccountId, |
Member
There was a problem hiding this comment.
So to maintain security, we need to have next information:
<account_id, validator_peer_id> + <peer_id sent to> signed with validator public key
when route from one existing peer to next peer, need to attach what have received before and then <peer_id sent to> + signature of the current peer_id
The problem is that self referential data structures don't work in Rust very well.
So data structure should be something like
struct AnnounceAccountRoute {
peer_id_sender: PeerId,
peer_id_receiver: PeerId,
}
struct AnnounceAccount {
account_id: AccountId
route: Vec<AnnounceAccountRoute>,
signatures: Vec<Signature>
}
and then methods to dissect route into parts and check signatures.
E.g. fn prefix() -> AnnounceAccount returns same AnnounceAccount but removing one item from route and signature - you can then check it's hash matches the last signature. or something like this.
Member
There was a problem hiding this comment.
Oh, and when you send it, you wrap existing with new route that records to whom you send and sign that.
Member
Author
There was a problem hiding this comment.
But the signers are account_id holders, not peer_id holders. On the other hand the route is created among peer_id not account_id, do we want full mapping between account_id, peer_id while making the route.
Remember that we can be connected to some peer and not have its account_id, in such case we can't verify such signature.
Member
Author
There was a problem hiding this comment.
But the signers are account_id holders, not peer_id holders. On the other hand the route is created among peer_id not account_id, do we want full mapping between account_id, peer_id while making the route.
Remember that we can be connected to some peer and not have its account_id, in such case we can't verify such signature.
Contributor
SkidanovAlex
left a comment
There was a problem hiding this comment.
There's no tests? How do we know any of those messages are sent, and that they are actually sent when we want them to be sent?
chain/client/src/client.rs
Outdated
| self.network_actor.send(NetworkRequests::AnnounceAccount(announce_account)); | ||
| NetworkClientResponses::NoResponse | ||
| } else { | ||
| NetworkClientResponses::Ban { ban_reason: ReasonForBan::InvalidSignature } |
Contributor
There was a problem hiding this comment.
Could also be invalid hash. Shall we return Result instead incheck_signature_account_announce, and propagate the error here?
Add routing information in NetworkInfo for testing.
Member
Author
@SkidanovAlex @ilblackdragon Already added several tests. |
ilblackdragon
approved these changes
Jul 27, 2019
ilblackdragon
added a commit
that referenced
this pull request
Aug 10, 2019
* Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * Add epoch hash to block headers (#1089) * add epoch hash to block header * rework runtime adapter API * address comments * Check validator signature (#1100) * check validator signature * ignore flaky test * fix (#1101) * Telemetry (#1090) * Add telemetry cargo, which sends RPC to given url. Add serde attributes to transaction to correctly serialize u128. Moved out information into a separate module called from client * Added signature to info telemetry. Default url for telemetry. Better showing mem and cpu * Add formatting for Option<BaseEncode>, fix comments * bump version to 0.2.6 (#1091) * Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * bump version to 0.2.6 * Update docker to have git version and use cache (#1092) * Disable test_kickout * Remove trailing slash in DEFAULT_TELEMETRY_URL * Adding node id to the info/telemetry * Return stake 3 epochs later (#1105) * Return stake 3 epochs later * remove print * fix test * remove clutch * Networking AccountId tracking (#1058) * Remove account id from handshake * Add AnnounceAccount + Communication between client and network on trigger Missing trigger condition + Routing table as mechanism to support conection through hops * Improve signed data on announce & check conditions * reformat announce account into simpler pieces * Handle client side. * Fix tests * Provide `get_epoch_offset` for mock runtime * Address small issues * Use hash for epoch instead of height * Verify signature from account announcement * Verify route while announcing account * nit * Fix mock test * Add missing verify * Update NewtorkInfo api for ease to use on tests * Add test for announce account. Add routing information in NetworkInfo for testing. * Exact cause for ban in invalid acc announcement. * Add test for announce account with different graph * Avoid extra clone and queries * Fix up merge conflict * Account deletion (#1107) * Account deletion implementation * Stakers can't be deleted, but must have more rent on their account (4 * epoch_length). Also added check_rent method to call after transaction execution to make sure there is still enough rent on the account * Actually delete data when deleting an account and test that * Add support for default values in runtime config (expected that we will put reasonable defaults in the binary), and fixed test_deserialize test / testnet.json * Moved helper functions for transactions into test-utils User trait * Reuse check_stake function from runtime and system * Address comments * Merge master into staging (#1116) * bump version to 0.2.6 (#1091) * Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * bump version to 0.2.6 * Update docker to have git version and use cache (#1092) * neartest.com has been retired (#1098) * Change how docker port mapping works for macos (#1086) * Apply Peter's fix to docker image start up code * fix port mapping in nodelib * fix #1042: Ban peer doesn't save to storage (#1093) * Add registers to Wasm bindings * Remove some unreachables (#1108) * Revert back to cranelift backend for now (#1109) Singlepass backend is crashing * Fix stake test (#1095) * Fix stake test * Check expected validators. * Specify expected validator order. * No need to move anymore. * Add slashing in validator manager (#1112) * Add slashing in validator manager * pass slashed_validators as a parameter to add_proposals * change runtime adapter to include slashing * refactor slashing * Finish kickout test after everything gets verified (#1123) * Finish kickout test after everything gets verified * Remove println * typo (#1124) * Remove merge conflict issue from store/lib.rs * Bump version to 0.2.7 * Fix test_deserialize issue * Enabled sending telemetry data over https (#1149) * Update testnet genesis (#1150) * HARD FORK: update testnet genesis * Dedup validators in get_epoch_producers * Correct test and reduce size of docker by removing files after docker is done * Zero out storage_used_at * Validator proposals fix (#1154) * Fix issue that validator proposals were not propagated to Block::produce * Speed up sync_state_stake_change and remove extra print * Update testnet genesis to 1859314 block * Hot fixes to syncing issues * Fix issue with orphan resolution * Fix genesis with fisherman to 0 for now * Fix rushed fix * Fix validator_join and remove warning * Fix total approvals formula when there are 2 validatos * Bump up the timelimits for announce_account tests * Check what happened on CI for the announce_account test * Add ignore to account_announce tests as they are currently not fully propagating account into. Will be fixing separtely
ilblackdragon
pushed a commit
that referenced
this pull request
Aug 29, 2019
* Release v0.2.7 (#1155) * Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * Add epoch hash to block headers (#1089) * add epoch hash to block header * rework runtime adapter API * address comments * Check validator signature (#1100) * check validator signature * ignore flaky test * fix (#1101) * Telemetry (#1090) * Add telemetry cargo, which sends RPC to given url. Add serde attributes to transaction to correctly serialize u128. Moved out information into a separate module called from client * Added signature to info telemetry. Default url for telemetry. Better showing mem and cpu * Add formatting for Option<BaseEncode>, fix comments * bump version to 0.2.6 (#1091) * Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * bump version to 0.2.6 * Update docker to have git version and use cache (#1092) * Disable test_kickout * Remove trailing slash in DEFAULT_TELEMETRY_URL * Adding node id to the info/telemetry * Return stake 3 epochs later (#1105) * Return stake 3 epochs later * remove print * fix test * remove clutch * Networking AccountId tracking (#1058) * Remove account id from handshake * Add AnnounceAccount + Communication between client and network on trigger Missing trigger condition + Routing table as mechanism to support conection through hops * Improve signed data on announce & check conditions * reformat announce account into simpler pieces * Handle client side. * Fix tests * Provide `get_epoch_offset` for mock runtime * Address small issues * Use hash for epoch instead of height * Verify signature from account announcement * Verify route while announcing account * nit * Fix mock test * Add missing verify * Update NewtorkInfo api for ease to use on tests * Add test for announce account. Add routing information in NetworkInfo for testing. * Exact cause for ban in invalid acc announcement. * Add test for announce account with different graph * Avoid extra clone and queries * Fix up merge conflict * Account deletion (#1107) * Account deletion implementation * Stakers can't be deleted, but must have more rent on their account (4 * epoch_length). Also added check_rent method to call after transaction execution to make sure there is still enough rent on the account * Actually delete data when deleting an account and test that * Add support for default values in runtime config (expected that we will put reasonable defaults in the binary), and fixed test_deserialize test / testnet.json * Moved helper functions for transactions into test-utils User trait * Reuse check_stake function from runtime and system * Address comments * Merge master into staging (#1116) * bump version to 0.2.6 (#1091) * Fix stake double return bug (#1084) * fix stake double return bug and update tests * use cache properly * properly implement stake returning * refactor test utils * fix gensis config * Fix block producer assignment (#1088) * fix block producer assignment * trying with test lock * tweak test parameters * bump version to 0.2.6 * Update docker to have git version and use cache (#1092) * neartest.com has been retired (#1098) * Change how docker port mapping works for macos (#1086) * Apply Peter's fix to docker image start up code * fix port mapping in nodelib * fix #1042: Ban peer doesn't save to storage (#1093) * Add registers to Wasm bindings * Remove some unreachables (#1108) * Revert back to cranelift backend for now (#1109) Singlepass backend is crashing * Fix stake test (#1095) * Fix stake test * Check expected validators. * Specify expected validator order. * No need to move anymore. * Add slashing in validator manager (#1112) * Add slashing in validator manager * pass slashed_validators as a parameter to add_proposals * change runtime adapter to include slashing * refactor slashing * Finish kickout test after everything gets verified (#1123) * Finish kickout test after everything gets verified * Remove println * typo (#1124) * Remove merge conflict issue from store/lib.rs * Bump version to 0.2.7 * Fix test_deserialize issue * Enabled sending telemetry data over https (#1149) * Update testnet genesis (#1150) * HARD FORK: update testnet genesis * Dedup validators in get_epoch_producers * Correct test and reduce size of docker by removing files after docker is done * Zero out storage_used_at * Validator proposals fix (#1154) * Fix issue that validator proposals were not propagated to Block::produce * Speed up sync_state_stake_change and remove extra print * Update testnet genesis to 1859314 block * Hot fixes to syncing issues * Fix issue with orphan resolution * Fix genesis with fisherman to 0 for now * Fix rushed fix * Fix validator_join and remove warning * Fix total approvals formula when there are 2 validatos * Bump up the timelimits for announce_account tests * Check what happened on CI for the announce_account test * Add ignore to account_announce tests as they are currently not fully propagating account into. Will be fixing separtely * Remove #update-as from dependencies (#1168) * Remove #nightshade from near-shell dependency (#1169) * Add print headers to state-viewer (#1171) * add print headers to state-viewer * more info * Fix bug in printing blocks (#1173) * add print headers to state-viewer * more info * fix * Allow state viewer to replay chain (#1175) * Allow state viewer to replay chain * add help * Fix incorrect epoch hash computation (#1214) * Fix incorrect epoch hash computation * fix another bug * bump version to 0.2.8 * use bs58 properly * fix tests * address comments * Fix compilatoin error
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Solving #1043