Skip to content
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

Detach MaxTraceableBlocks from MaxValidUntilBlockIncrement #2027

Closed
roman-khimov opened this issue Oct 27, 2020 · 1 comment
Closed

Detach MaxTraceableBlocks from MaxValidUntilBlockIncrement #2027

roman-khimov opened this issue Oct 27, 2020 · 1 comment
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@roman-khimov
Copy link
Contributor

Summary or problem description
At the moment our MaxTraceableBlocks constant it tied to Transaction.MaxValidUntilBlockIncrement, although they're very different in their behavior. MaxTraceableBlocks limits depth of possible contract's GetBlock/GetTransaction queries, while MaxValidUntilBlockIncrement restricts transaction validity period effectively limiting its P2P broadcasting and block inclusion.

MaxTraceableBlocks should be fixed in #2026 (it's written with that meaning of MaxValidUntilBlockIncrement in mind and 200K is a good value for it), but as MaxValidUntilBlockIncrement is mostly about block inclusion I think we can improve it by using much lower values (which reminds of #995 also). This would allow for any transaction to get a result in a reasonable time --- either it gets accepted into the block or it's not and it'll never be (so things like #1969 could be resolved more easily).

It could be improved even further by using lower increments when creating a transaction. In normal situation I'd expect a transaction to get into the block quickly, 1-2 blocks away from current height. Or at least expect one of the validators to include it. So some ValidatorsCount + 3 increment value should be just fine for ordinary transactions. And this low value would give the user an appropriate feedback even quicker, either his transaction gets in in the next 10 blocks (2 and half minutes) or it never does and he needs to send a new one. It also mitigates replay attacks from #1502, the transaction could only be replayed within a quite short window of time.

Do you have any solution you want to propose?
Leave MaxTraceableBlocks at 200K and set MaxValidUntilBlockIncrement to 240 (1 hour with 15 seconds intervals). Then use even lower values (like 10) by default in the wallet code.

Neo Version

  • Neo

Where in the software does this update applies to?

  • Ledger
  • Network Policy
  • P2P (TCP)
@roman-khimov roman-khimov added the Discussion Initial issue state - proposed but not yet accepted label Oct 27, 2020
@roman-khimov
Copy link
Contributor Author

Fixed by #2042.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

No branches or pull requests

1 participant