Skip to content
This repository was archived by the owner on Jan 9, 2026. It is now read-only.

Fix div by zero in concat gas model#1317

Merged
jmcardon merged 1 commit intomasterfrom
gr/fix-concat-div-0
Nov 13, 2023
Merged

Fix div by zero in concat gas model#1317
jmcardon merged 1 commit intomasterfrom
gr/fix-concat-div-0

Conversation

@0xd34df00d
Copy link
Copy Markdown
Contributor

PR checklist:

  • Test coverage for the proposed changes
  • PR description contains example output from repl interaction or a snippet from unit test output
  • Documentation has been updated if new natives or FV properties have been added. To generate new documentation, issue cabal run tests. If they pass locally, docs are generated.
  • Any changes that could be relevant to users have been recorded in the changelog
  • In case of changes to the Pact trace output (pact -t), make sure pact-lsp is in sync.

Additionally, please justify why you should or should not do the following:

  • Confirm replay/back compat
  • Benchmark regressions
  • (For Kadena engineers) Run integration-tests against a Chainweb built with this version of Pact

GSortFieldLookup i -> "GSortFieldLookup:" <> pretty i
GConcatenation i j -> "GConcatenation:" <> pretty i <> colon <> pretty j
GTextConcatenation nChars nStrings -> "GTextConcatenation:" <> pretty nChars <> colon <> pretty nStrings
GTextConcatenation nChars nStrings fixupDiv -> "GTextConcatenation:" <> pretty nChars <> colon <> pretty nStrings <> colon <> pretty fixupDiv
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assumes changing this Pretty instance doesn't participate in on-chain computations. I skimmed through the code and found no evidence it does, but worth double-checking.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about this, @jmcardon ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It does not. Gas logs are not part of txlog outputs that get hashed.

(expect "calling (concat strings_1000_2000)" 151 (env-gas))

;; Test issue #1316
(expect "concat gassing works on empty list" "" (concat []))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@jmcardon jmcardon merged commit 6759cc8 into master Nov 13, 2023
@0xd34df00d 0xd34df00d deleted the gr/fix-concat-div-0 branch November 13, 2023 16:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants