Skip to content

Conversation

@michaelfolkson
Copy link
Contributor

At the very least I need to add hashtags (I couldn't find the hashtags to link to specific sections of previous newsletters). There may be other stuff I've missed too.

However, I think we need a Simplicity topics page as it has cropped up in the newsletter a number of times now and so hopefully this is at least a start.

@harding
Copy link
Collaborator

harding commented Aug 3, 2020

Concept ACK. Thanks!

I'd prefer a more detailed excerpt/detailed_summary, perhaps including information like:

  • Planned for Elements
  • Not proposed for Bitcoin directly in the near future (e.g. the next 24 months)
  • Easier proofs of correctness than Script
  • Maybe better resource accounting / better management of DoS risks
  • Native merklized scripting
  • Risks, e.g. more Ethereum-like losses due to poorly written contracts
  • Costs, e.g. non-jetted scripts might be more expensive than current scripts
  • More details about what jets are and why they're useful

In short, text that convinces me that this is worth considering (but without hiding any likely downsides).

@michaelfolkson Let me know if the anchor linking instructions in harding#15 (comment) are insufficient to address the links on this PR as well.

@bitschmidty
Copy link
Contributor

Concept ACK as well! I could have used this link last week as well

@jnewbery
Copy link
Contributor

jnewbery commented Aug 3, 2020

Thanks for this @michaelfolkson . Will review once you've addressed @harding's review comments.

@jnewbery
Copy link
Contributor

@michaelfolkson - are you planning to update this PR?

@michaelfolkson
Copy link
Contributor Author

Yes I will work on this after I've finished this month's transcripts section. Thanks for the reminder and apologies for delay.

@michaelfolkson
Copy link
Contributor Author

michaelfolkson commented Sep 2, 2020

Pushed an additional commit that addresses most if not all of @harding suggestions and adds the correct links to prior newsletters.

[edit: The content is ready to be reviewed (I think) but checks are failing so evidently I still need to address some formatting issues.]

Copy link
Collaborator

@harding harding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an improvement, thanks! Left some comments. Looks like tests are failing because the intro paragraph is over 500 characters.

@jnewbery
Copy link
Contributor

@michaelfolkson - quick reminder that there are some outstanding review comments on this and the build is failing because the intro paragraph is too long. Are you planning to work on this again?

@michaelfolkson
Copy link
Contributor Author

@jnewbery: Yes I'll work on this Thursday and push update then (after Bitcoin Core PR review club) if that is ok.

@michaelfolkson michaelfolkson force-pushed the 2020-08-topics-simplicity branch from 50dec48 to 0e7e2c2 Compare October 15, 2020 17:33
@michaelfolkson
Copy link
Contributor Author

michaelfolkson commented Oct 15, 2020

Pushed latest changes but builds are failing (I haven't built locally yet). Will investigate.

Looks like tests are failing because the intro paragraph is over 500 characters.

Is the limit for the intro paragraph 500 characters including spaces or excluding spaces? I think it is 500 characters including spaces...

@michaelfolkson michaelfolkson force-pushed the 2020-08-topics-simplicity branch 2 times, most recently from 3b966f4 to 9fec4bb Compare October 15, 2020 18:19
@harding
Copy link
Collaborator

harding commented Oct 15, 2020

@michaelfolkson

Is the limit for the intro paragraph 500 characters including spaces or excluding spaces? I think it is 500 characters including spaces...

Including spaces (but not the indentation).

@michaelfolkson michaelfolkson force-pushed the 2020-08-topics-simplicity branch from 9fec4bb to e5546b8 Compare October 16, 2020 11:56
@michaelfolkson
Copy link
Contributor Author

Checks are passing now. It didn't like [SIGHASH_ANYPREVOUT][topic SIGHASH_ANYPREVOUT]. I don't know if this was because ANYPREVOUT was upper case or because ANYPREVOUT is an alias of NOINPUT and you need to use SIGHASH_NOINPUT as the topic. Anyway ready to be re-reviewed. I think I have addressed all of @harding comments.

@jnewbery
Copy link
Contributor

I don't know if this was because ANYPREVOUT was upper case or because ANYPREVOUT is an alias of NOINPUT and you need to use SIGHASH_NOINPUT as the topic.

Yes, you need to use the shortname of the topic page, or the title if there's no shortname. The title in this case is SIGHASH_NOINPUT.

Topics: suggested edits to Simplicity topic
@michaelfolkson
Copy link
Contributor Author

Merged in some more @harding suggested edits. I think this is at final review stage (or at least very close to it).

@michaelfolkson
Copy link
Contributor Author

A reminder that this is ready to be reviewed. Latest @harding edits are merged.

@harding
Copy link
Collaborator

harding commented Nov 9, 2020

ACK 931c5c1

If additional changes are requested by other people, I think maybe the simplicity.pdf link should be moved from the see_also section to the primary_sources section. I don't think that's particularly important, though, and I feel bad about not re-reviewing this sooner as it is.

@jnewbery jnewbery merged commit 4448177 into bitcoinops:master Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants