Remove max block time param #3366

sug0 · 2024-06-04T14:47:22Z

Describe your changes

Remove the max_expected_time_per_block genesis parameter.

Indicate on which release or other PRs this topic is based on

v0.39.0

Checklist before merging to `draft`

I have added a changelog
Git history is in acceptable state

codecov · 2024-06-04T15:15:05Z

Codecov Report

Attention: Patch coverage is 81.21547% with 68 lines in your changes missing coverage. Please review.

Project coverage is 54.01%. Comparing base (879a326) to head (48f58aa).
Report is 4 commits behind head on main.

Files	Patch %	Lines
crates/sdk/src/queries/shell.rs	0.00%	26 Missing ⚠️
crates/sdk/src/masp.rs	0.00%	21 Missing ⚠️
crates/parameters/src/lib.rs	95.18%	13 Missing ⚠️
crates/sdk/src/rpc.rs	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3366      +/-   ##
==========================================
+ Coverage   53.92%   54.01%   +0.09%     
==========================================
  Files         317      317              
  Lines      107575   107839     +264     
==========================================
+ Hits        58011    58254     +243     
- Misses      49564    49585      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

crates/sdk/src/queries/shell.rs

crates/parameters/src/lib.rs

yito88 · 2024-06-05T15:52:13Z

I tried IBC e2e tests with the updated Hermes and IBC shielding transfer failed due to MASP VP error: Native VP error: MASP transaction is expired.
The current height + 7 (where default timeout was 3599 sec, max_block_time was 450 in namada_sdk::masp::gen_shielded_transfer) was set to the MASP transaction expiration. The height elapsed in the IBC packet relaying.

tzemanovic · 2024-06-07T12:40:10Z

crates/parameters/src/lib.rs

+    let block_timestamps = {
+        let mut ts = Vec::with_capacity(
+            usize::try_from({
+                // NB: cap the allocation to 16


nit: let's note this cap in the rustdoc for this fn and estimate_max_block_time_from_blocks_and_params

yito88 · 2024-06-07T13:46:13Z

The IBC E2E tests in CI failed when creating channels. I'm investigating it...

yito88 · 2024-06-07T15:15:23Z

hmm, flaky tests... These tests didn't fail locally. After some retries, they passed.

sug0 · 2024-06-07T21:17:38Z

hmm, flaky tests... These tests didn't fail locally. After some retries, they passed.

@yito88 I suppose it's a matter of decreasing the max block time, to increase the amount of delta blocks in the masp tx expiration

brentstone · 2024-06-10T14:11:29Z

Is this ready for draft now?

* tiago/remove-max-block-time-param: update Hermes Changelog for #3366 Add additional test coverage for max expected block times Import `namada_storage` with `testing` feat Avoid estimating block time of zero Test max block time estimates Insert mock block headers into test storage Split out `estimate_max_block_time_from_parameters` Remove max expected block time param Retrieve max block time estimate from new rpc method in masp Add max block duration estimate rpc method Read epochs per year param Implement date time subtraction Add block header Namada rpc handler

sug0 · 2024-06-13T15:10:58Z

still messing around with timeout values to avoid flakes in ci

sug0 · 2024-06-14T10:09:18Z

@brentstone I think this is less flakey now. should be ok to merge

* tiago/remove-max-block-time-param: Changelog for #3366 Adjust IBC timeout values Clue IBC max block time estimate from Namada params Decrease max block time estimate to 1m update Hermes Add additional test coverage for max expected block times Import `namada_storage` with `testing` feat Avoid estimating block time of zero Test max block time estimates Insert mock block headers into test storage Split out `estimate_max_block_time_from_parameters` Remove max expected block time param Retrieve max block time estimate from new rpc method in masp Add max block duration estimate rpc method Read epochs per year param Implement date time subtraction Add block header Namada rpc handler

sug0 added ledger IBC MASP SDK breaking:genesis breaking:state labels Jun 4, 2024

sug0 marked this pull request as draft June 4, 2024 15:20

sug0 requested review from murisi, yito88 and grarco June 5, 2024 12:29

sug0 marked this pull request as ready for review June 5, 2024 12:29

sug0 added a commit that referenced this pull request Jun 5, 2024

Changelog for #3366

fc02f3c

grarco reviewed Jun 5, 2024

View reviewed changes

crates/sdk/src/queries/shell.rs Show resolved Hide resolved

grarco reviewed Jun 5, 2024

View reviewed changes

crates/parameters/src/lib.rs Outdated Show resolved Hide resolved

sug0 added a commit that referenced this pull request Jun 6, 2024

Changelog for #3366

da17c33

sug0 force-pushed the tiago/remove-max-block-time-param branch from fc02f3c to da17c33 Compare June 6, 2024 08:03

sug0 added 12 commits June 6, 2024 10:12

Add block header Namada rpc handler

ca10451

Implement date time subtraction

493e453

Read epochs per year param

3cca791

Add max block duration estimate rpc method

93112ea

Retrieve max block time estimate from new rpc method in masp

64d974d

Remove max expected block time param

8bb9f8c

Split out estimate_max_block_time_from_parameters

7481d60

Insert mock block headers into test storage

6b71302

Test max block time estimates

996c48e

Avoid estimating block time of zero

beadc81

Import namada_storage with testing feat

eebf5d9

Add additional test coverage for max expected block times

2164904

sug0 requested a review from tzemanovic June 6, 2024 21:24

tzemanovic reviewed Jun 7, 2024

View reviewed changes

tzemanovic previously approved these changes Jun 7, 2024

View reviewed changes

brentstone added ready for draft and removed ready for draft labels Jun 10, 2024

brentstone added the merged to draft label Jun 10, 2024

yito88 and others added 3 commits June 13, 2024 13:40

update Hermes

2434e73

Decrease max block time estimate to 1m

c4d788d

Clue IBC max block time estimate from Namada params

db67411

sug0 added a commit that referenced this pull request Jun 13, 2024

Changelog for #3366

2b6d0eb

sug0 dismissed tzemanovic’s stale review via 2b6d0eb June 13, 2024 12:54

sug0 force-pushed the tiago/remove-max-block-time-param branch from 7ed0ef6 to 2b6d0eb Compare June 13, 2024 12:54

sug0 marked this pull request as draft June 13, 2024 15:10

sug0 added a commit that referenced this pull request Jun 13, 2024

Changelog for #3366

df9c095

sug0 force-pushed the tiago/remove-max-block-time-param branch from 2b6d0eb to df9c095 Compare June 13, 2024 15:11

sug0 added 2 commits June 14, 2024 10:26

Adjust IBC timeout values

17adaeb

Changelog for #3366

48f58aa

sug0 force-pushed the tiago/remove-max-block-time-param branch from df9c095 to 48f58aa Compare June 14, 2024 09:26

sug0 marked this pull request as ready for review June 14, 2024 10:10

brentstone merged commit f479d3d into main Jul 5, 2024
17 of 19 checks passed

brentstone deleted the tiago/remove-max-block-time-param branch July 5, 2024 21:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove max block time param #3366

Remove max block time param #3366

sug0 commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

yito88 commented Jun 5, 2024 •

edited

Loading

tzemanovic Jun 7, 2024

yito88 commented Jun 7, 2024

yito88 commented Jun 7, 2024

sug0 commented Jun 7, 2024

brentstone commented Jun 10, 2024

sug0 commented Jun 13, 2024

sug0 commented Jun 14, 2024 •

edited

Loading

Remove max block time param #3366

Remove max block time param #3366

Conversation

sug0 commented Jun 4, 2024 • edited Loading

Describe your changes

Indicate on which release or other PRs this topic is based on

Checklist before merging to draft

codecov bot commented Jun 4, 2024 • edited Loading

Codecov Report

yito88 commented Jun 5, 2024 • edited Loading

tzemanovic Jun 7, 2024

Choose a reason for hiding this comment

yito88 commented Jun 7, 2024

yito88 commented Jun 7, 2024

sug0 commented Jun 7, 2024

brentstone commented Jun 10, 2024

sug0 commented Jun 13, 2024

sug0 commented Jun 14, 2024 • edited Loading

sug0 commented Jun 4, 2024 •

edited

Loading

Checklist before merging to `draft`

codecov bot commented Jun 4, 2024 •

edited

Loading

yito88 commented Jun 5, 2024 •

edited

Loading

sug0 commented Jun 14, 2024 •

edited

Loading