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

Update policy - Add maximum witness size (in bytes) #1020

Merged
merged 12 commits into from Aug 21, 2019

Conversation

@shargon
Copy link
Member

commented Aug 12, 2019

Close #979

Max witness according to 10/10 multisignature

@vncoelho
Copy link
Member

left a comment

Is it ready @shargon?
I thought it would go to the policy. This PR is adjusting these limits, right?

@igormcoelho could take a look more careful.

@shargon

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

Yes, Is ready

shargon added 2 commits Aug 13, 2019
@igormcoelho
Copy link
Contributor

left a comment

10/10 may be more reasonable... around 1000 bytes.
[EDIT]: my mistake.. it was 1003 bytes 😂 I'll round up to 1024, which is more "common" number ;)

igormcoelho added 2 commits Aug 14, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Aug 15, 2019

Codecov Report

Merging #1020 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1020   +/-   ##
=======================================
  Coverage   61.35%   61.35%           
=======================================
  Files         195      195           
  Lines       13405    13405           
=======================================
  Hits         8224     8224           
  Misses       5181     5181
Impacted Files Coverage Δ
neo/Network/P2P/Payloads/Witness.cs 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cc961b...00deb5d. Read the comment docs.

@igormcoelho

This comment has been minimized.

Copy link
Contributor

commented on neo.UnitTests/Network/P2P/Payloads/UT_Witness.cs in cbc5e86 Aug 15, 2019

Nice test!

@igormcoelho

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

@shargon, we need to expose this max witness size limit in a public (constant) variable, so we can also reuse it on Max Block Size

Please take a look to see if it's reasonable:
#953 (comment)

@shargon shargon requested review from igormcoelho and vncoelho Aug 20, 2019

@shargon

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

@igormcoelho I prefer the real size, is easy to compute.

shargon added 2 commits Aug 20, 2019
@lock9
lock9 approved these changes Aug 20, 2019
@vncoelho
Copy link
Member

left a comment

Nice comments and UTs.

@shargon shargon merged commit b1a4d29 into neo-project:master Aug 21, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@shargon shargon deleted the shargon:max-witness branch Aug 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.