May 2022 Hardfork by cpacia · Pull Request #499 · gcash/bchd

cpacia · 2022-04-16T16:16:04Z

This PR upgrades BCHD to support the May 2022 hardfork

Tasks:

Create 64 bit integer and new opcode script flags
Create 64 bit integer implementation
Create 64 bit integer tests
Create opcode implementations
Create opcode tests
Update params with the activation timestamp and set the script flags after activation
Create activation test
Sync with hardfork testnet

Spec:

64 Bit Integers: https://gitlab.com/GeneralProtocols/research/chips/-/blob/master/CHIP-2021-02-Bigger-Script-Integers.md
Native Introspection Opcodes: https://gitlab.com/GeneralProtocols/research/chips/-/blob/master/CHIP-2021-02-Add-Native-Introspection-Opcodes.md

This commit adds a flag to the script engine to use 64 bit integers and updates the ScriptNum class to optionally support 64 bit integers. This is the first part of the May 2022 hardfork.

blockchain/fullblocks_test.go

emergent-reasons · 2022-04-18T09:53:05Z

blockchain/fullblocktests/generate.go

+		tx1 := createSpendTx(outs[0], 0)
+		builder := txscript.NewScriptBuilder().
+			AddOp(txscript.OP_1).
+			AddOp(txscript.OP_ADD).
+			AddInt64(1152921504606846976).
+			AddOp(txscript.OP_EQUAL)
+		redeemScript, err := builder.Script()
+		if err != nil {
+			panic(err)
+		}
+
+		addr, err := bchutil.NewAddressScriptHash(redeemScript, &chaincfg.RegressionNetParams)
+		if err != nil {
+			panic(err)
+		}
+		script, err := txscript.PayToAddrScript(addr)
+		if err != nil {
+			panic(err)
+		}
+		tx1.TxOut[0].PkScript = script
+
+		block.AddTransaction(tx1)
+		unsortedTxs = append(unsortedTxs, bchutil.NewTx(tx1))
+
+		// Spend from tx1
+		so1 := &spendableOut{
+			amount: bchutil.Amount(tx1.TxOut[0].Value),
+			prevOut: wire.OutPoint{
+				Hash:  tx1.TxHash(),
+				Index: 0,
+			},
+		}
+
+		tx2 := createSpendTx(so1, 0)
+		scriptSig, err := txscript.NewScriptBuilder().
+			AddInt64(1152921504606846975).
+			AddData(redeemScript).Script()


Ah shit sorry. This suggestion (deleted) just came out as a big red block. It just has 3 line changes:

sixtyBitInt := int64(1152921504606846976) ... AddInt64(sixtyBitInt). ... AddInt64(sixtyBitInt - 1).

I suggest it because it took me a while to realize the two scripts are connected.

Could add an op_1 op_mul too (or in replacement of add) if you want to exercise both 64 and the re-enabled op together.

emergent-reasons · 2022-04-18T11:13:36Z

Needs OP_MUL to be re-enabled / implemented with appropriate overflow mechanic.

(resolved now)

emergent-reasons · 2022-04-19T04:17:37Z

Do you want me to take a shot at re-documenting these two?

scriptNum
makeScriptNum

Also do these need a documentation update since there is going to be a boundary between 32 vs 64 script int behavior? e.g. in script_tests.json. I didn't check for raw numbers that may be asserting current/past behavior on 32 bit script ints.

 ["2147483648 0 ADD", "NOP", "P2SH,STRICTENC", "UNKNOWN_ERROR", "arithmetic operands must be in range [-2^31...2^31] "],
 ["-2147483648 0 ADD", "NOP", "P2SH,STRICTENC", "UNKNOWN_ERROR", "arithmetic operands must be in range [-2^31...2^31] "],

emergent-reasons · 2022-04-19T06:35:12Z

Finished one pass of review.

opcode_test.go took me some time to figure out how all the pieces fit together.
op_mul which is added now
some minor fixes
a few places where I wasn't confident about the test because I didn't understand the logic.

cpacia · 2022-04-19T18:16:56Z

Do you want me to take a shot at re-documenting these two?

scriptNum

makeScriptNum

Also do these need a documentation update since there is going to be a boundary between 32 vs 64 script int behavior? e.g. in script_tests.json. I didn't check for raw numbers that may be asserting current/past behavior on 32 bit script ints.
 ["2147483648 0 ADD", "NOP", "P2SH,STRICTENC", "UNKNOWN_ERROR", "arithmetic operands must be in range [-2^31...2^31] "],
 ["-2147483648 0 ADD", "NOP", "P2SH,STRICTENC", "UNKNOWN_ERROR", "arithmetic operands must be in range [-2^31...2^31] "],

sure

emergent-reasons · 2022-04-20T09:26:40Z

sure

Made a first draft only for scriptnum and makeScriptNum here: #501

It's set so you can make commits directly or if you mention any changes, I can make them.

zquestz · 2022-04-30T03:28:38Z

blockchain/thresholdstate.go

 import (
 	"fmt"
+	"github.com/gcash/bchd/chaincfg"



NIT: extra newline.

zquestz · 2022-04-30T03:29:45Z

chaincfg/genesis.go

+// for the main network.
+var testNet4GenesisMerkleRoot = genesisMerkleRoot
+
+// testNet3GenesisBlock defines the genesis block of the block chain which


testNet4GenesisBlock

zquestz · 2022-04-30T03:30:25Z

chaincfg/params.go

 }

+// TestNet4Params defines the network parameters for the test Bitcoin network
+// (version 3).  Not to be confused with the regression test network, this


(version 4).

zquestz · 2022-04-30T03:30:58Z

chaincfg/params.go

+	// Chain parameters
+	GenesisBlock:  &testNet4GenesisBlock,
+	GenesisHash:   &testNet4GenesisHash,
+	PowLimit:      testNet3PowLimit,


Should we have another variable for the PowLimit? Odd to use testNet3 values here.

zquestz

This looks amazing. Thank you for doing this work @cpacia.

Have we synced this against both testnet3 and testnet4?

emergent-reasons · 2022-04-30T09:45:13Z

This looks amazing. Thank you for doing this work @cpacia.

Have we synced this against both testnet3 and testnet4?

im_uname says it has been tested against testnet4. The problem is that it didn't fail before op_mul was implemented in this PR. So it probably still needs to be synced against some op_mul tx.

cpacia · 2022-04-30T14:57:19Z

Yeah it synced to testnet4 but it was odd that it did because OP_MUL wasn't activated when I did it. Are there OP_MUL transactions on that chain?

emergent-reasons · 2022-04-30T15:02:51Z

Yeah it synced to testnet4 but it was odd that it did because OP_MUL wasn't activated when I did it. Are there OP_MUL transactions on that chain?

These are supposed to have op_mul:

cpacia changed the title ~~Add Script64BitIntegers script flag~~ May 2022 Hardfork Apr 16, 2022

Add Script64BitIntegers script flag

6c3a99a

This commit adds a flag to the script engine to use 64 bit integers and updates the ScriptNum class to optionally support 64 bit integers. This is the first part of the May 2022 hardfork.

cpacia force-pushed the may2022hardfork branch from e2cc0b3 to 6c3a99a Compare April 16, 2022 16:18

cpacia added 9 commits April 16, 2022 14:51

Add ScriptNumber 64 bit tests

f979722

Create native introspection opcodes

ce0375d

Check for script flag in new opcodes

d32f4fb

Update script_tests.json

e50130d

Add opcode tests

15fc441

Add fork activation

7d98616

Add activation tests

a882bab

Create testnet4 params

c14737b

Bump version

cb76167

cpacia added the needs review Needs to be reviewed by the maintainers label Apr 18, 2022

Copy opcode slices before returning

66553d5

emergent-reasons reviewed Apr 18, 2022

View reviewed changes

blockchain/fullblocks_test.go Outdated Show resolved Hide resolved

emergent-reasons reviewed Apr 18, 2022

View reviewed changes

cpacia added 2 commits April 18, 2022 20:19

Add op_mul and fix overflow handling

14014a9

Disable op_mul before fork

81c7b0f

update scriptnum and makeScriptNum documentation for 64-bit cases (#501)

b6ec13b

zquestz reviewed Apr 30, 2022

View reviewed changes

blockchain/thresholdstate.go Outdated

import (

"fmt"

"github.com/gcash/bchd/chaincfg"

Copy link

Contributor

zquestz Apr 30, 2022

Choose a reason for hiding this comment

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

NIT: extra newline.

zquestz reviewed Apr 30, 2022

View reviewed changes

zquestz approved these changes Apr 30, 2022

View reviewed changes

Fix typos

40ca3f1

Klakurka mentioned this pull request Apr 30, 2022

Upgrade BCHD to 0.19.0 PayButton/paybutton-server#11

Closed

cpacia removed the needs review Needs to be reviewed by the maintainers label May 5, 2022

cpacia merged commit 58741b0 into master May 5, 2022

ygaberman-px mentioned this pull request Jun 20, 2023

Bump repo from v0.17.0 -> v0.19.0 paxosglobal/bchd#4

Merged

Conversation

cpacia commented Apr 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

emergent-reasons Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emergent-reasons commented Apr 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emergent-reasons commented Apr 19, 2022

Uh oh!

emergent-reasons commented Apr 19, 2022

Uh oh!

cpacia commented Apr 19, 2022

Uh oh!

emergent-reasons commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zquestz Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

zquestz Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

zquestz Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

zquestz Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

zquestz left a comment

Choose a reason for hiding this comment

Uh oh!

emergent-reasons commented Apr 30, 2022

Uh oh!

cpacia commented Apr 30, 2022

Uh oh!

emergent-reasons commented Apr 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cpacia commented Apr 16, 2022 •

edited

Loading

emergent-reasons Apr 18, 2022 •

edited

Loading

emergent-reasons commented Apr 18, 2022 •

edited

Loading

emergent-reasons commented Apr 20, 2022 •

edited

Loading