-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add eth outbound queue and system pallets #742
base: master
Are you sure you want to change the base?
Conversation
WASM runtime size check:Compared to target branchdancebox runtime: 1412 KB (no changes) ✅ flashbox runtime: 832 KB (no changes) ✅ dancelight runtime: 2056 KB (no changes) ✅ container chain template simple runtime: 1088 KB (no changes) ✅ container chain template frontier runtime: 1388 KB (no changes) ✅ |
…egate-message-origin
Coverage Report@@ Coverage Diff @@
## master tomasz-generic-aggregate-message-origin +/- ##
===========================================================================
- Coverage 65.44% 65.16% -0.28%
+ Files 309 310 +1
+ Lines 53985 54234 +249
===========================================================================
+ Hits 35328 35339 +11
+ Misses 18657 18895 +238
|
@@ -14,6 +14,10 @@ | |||
// You should have received a copy of the GNU General Public License | |||
// along with Tanssi. If not, see <http://www.gnu.org/licenses/> | |||
|
|||
use snowbridge_core::outbound::Fee; | |||
use snowbridge_core::outbound::SendError; |
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.
FMT
use frame_support::ensure; | ||
use frame_support::traits::{Defensive, ProcessMessage, ProcessMessageError}; | ||
use frame_support::weights::WeightMeter; | ||
use snowbridge_pallet_outbound_queue::MessageLeaves; |
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.
FMT again
|
||
#[pallet::call_index(2)] | ||
#[pallet::weight(T::WeightInfo::force_inject_slash())] | ||
pub fn root_test_send_msg_to_eth( |
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.
we need to bench this
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.
It's only for testing, and ensure_root, what's the point of benchmarking it?
type Balance = Balance; | ||
type WeightToFee = WeightToFee; | ||
type WeightInfo = (); | ||
//type WeightInfo = crate::weights::snowbridge_pallet_outbound_queue::WeightInfo<Runtime>; |
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.
put proper weight info benchmarked
type GetAggregateMessageOrigin = GetAggregateMessageOrigin; | ||
type MessageQueue = MessageQueue; | ||
type Decimals = ConstU8<12>; | ||
type MaxMessagePayloadSize = ConstU32<2048>; |
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.
it seems to be this will be not enough, maybe we should double it.
in the case of 100 validators, 32 byte per validator account * 100 is already 3200...
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.
We decided not to do this and instead send the message in batches, correct?
type Helper = benchmark_helper::EthSystemBenchHelper; | ||
type DefaultPricingParameters = Parameters; | ||
type InboundDeliveryCost = (); | ||
//type InboundDeliveryCost = EthereumInboundQueue; |
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.
same thing with benchmarks
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 will do benchmarks when the PR is ready, as they are time consuming
Upstream changes look good, waiting for a potential PR opening and approval |
No description provided.