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

protocol/vm: remove the dependency on protocol/bc #797

Merged
merged 5 commits into from Mar 23, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/generator/generator_test.go
Expand Up @@ -83,7 +83,7 @@ func TestGetAndAddBlockSignatures(t *testing.T) {
testutil.FatalErr(t, err)
}

err = vm.VerifyBlockHeader(&tip.BlockHeader, block)
err = vm.Verify(bc.NewBlockVMContext(block, tip.ConsensusProgram, block.Witness))
if err != nil {
testutil.FatalErr(t, err)
}
Expand Down
4 changes: 2 additions & 2 deletions protocol/bc/tx_test.go
Expand Up @@ -22,8 +22,8 @@ func TestTxHashes(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if len(hashes.VMContexts) != len(c.txdata.Inputs) {
t.Errorf("case %d: len(hashes.VMContexts) = %d, want %d", i, len(hashes.VMContexts), len(c.txdata.Inputs))
if len(hashes.SpentOutputIDs) != len(c.txdata.Inputs) {
t.Errorf("case %d: len(hashes.SpentOutputIDs) = %d, want %d", i, len(hashes.SpentOutputIDs), len(c.txdata.Inputs))
}
if c.hash != hashes.ID {
t.Errorf("case %d: got txid %x, want %x", i, hashes.ID[:], c.hash[:])
Expand Down
15 changes: 3 additions & 12 deletions protocol/bc/txhashes.go
Expand Up @@ -11,8 +11,8 @@ type (
ID Hash
ExpirationMS uint64
}
SpentOutputIDs []Hash // one per old-style Input. Non-spend inputs are blank hashes.
VMContexts []*VMContext // one per old-style Input
SpentOutputIDs []Hash // one per old-style Input. Non-spend inputs are blank hashes.
SigHashes []Hash // one per old-style Input.
}

// ResultInfo contains information about each result in a transaction header.
Expand All @@ -24,17 +24,8 @@ type (
SourcePos uint64 // the position within the source entry of this output's value
RefDataHash Hash // contents of the result entry's data field (which is a hash of the source refdata, when converting from old-style transactions)
}

VMContext struct {
TxRefDataHash Hash
RefDataHash Hash
TxSigHash Hash
OutputID *Hash
EntryID Hash
NonceID *Hash
}
)

func (t TxHashes) SigHash(n uint32) Hash {
return t.VMContexts[n].TxSigHash
return t.SigHashes[n]
}
33 changes: 7 additions & 26 deletions protocol/bc/txtransaction.go
Expand Up @@ -50,8 +50,8 @@ func ComputeTxHashes(oldTx *TxData) (hashes *TxHashes, err error) {
}
}

hashes.VMContexts = make([]*VMContext, len(oldTx.Inputs))
hashes.SpentOutputIDs = make([]Hash, len(oldTx.Inputs))
hashes.SigHashes = make([]Hash, len(oldTx.Inputs))

for entryID, ent := range entries {
switch ent := ent.(type) {
Expand All @@ -73,41 +73,22 @@ func ComputeTxHashes(oldTx *TxData) (hashes *TxHashes, err error) {
hashes.Issuances = append(hashes.Issuances, iss)

case *Issuance:
vmc := newVMContext(entryID, hashes.ID, header.body.Data, ent.body.Data)
vmc.NonceID = &ent.body.Anchor
hashes.VMContexts[ent.Ordinal()] = vmc
hashes.SigHashes[ent.Ordinal()] = makeSigHash(entryID, hashes.ID)

case *Spend:
vmc := newVMContext(entryID, hashes.ID, header.body.Data, ent.body.Data)
vmc.OutputID = &ent.body.SpentOutput
hashes.VMContexts[ent.Ordinal()] = vmc
hashes.SigHashes[ent.Ordinal()] = makeSigHash(entryID, hashes.ID)
hashes.SpentOutputIDs[ent.Ordinal()] = ent.body.SpentOutput
}
}

return hashes, nil
}

// populates the common fields of a VMContext for an Entry, regardless of whether
// that Entry is a Spend or an Issuance
func newVMContext(entryID, txid, txData, inpData Hash) *VMContext {
vmc := new(VMContext)

// TxRefDataHash
vmc.TxRefDataHash = txData

// RefDataHash (input-specific)
vmc.RefDataHash = inpData

// EntryID
vmc.EntryID = entryID

// TxSigHash
func makeSigHash(entryID, txID Hash) (hash Hash) {
hasher := sha3pool.Get256()
defer sha3pool.Put256(hasher)
hasher.Write(entryID[:])
hasher.Write(txid[:])
hasher.Read(vmc.TxSigHash[:])

return vmc
hasher.Write(txID[:])
hasher.Read(hash[:])
return hash
}
89 changes: 89 additions & 0 deletions protocol/bc/vmcontext.go
@@ -0,0 +1,89 @@
package bc

import (
"bytes"

"chain/protocol/vm"
)

func NewBlockVMContext(block *Block, prog []byte, args [][]byte) *vm.Context {
blockHash := block.Hash().Bytes()
return &vm.Context{
VMVersion: 1,
Code: prog,
Arguments: args,

BlockHash: &blockHash,
BlockTimeMS: &block.TimestampMS,
NextConsensusProgram: &block.ConsensusProgram,
}
}

func NewTxVMContext(tx *Tx, index uint32, prog Program, args [][]byte) *vm.Context {
var (
txSigHash = tx.SigHash(index).Bytes()
numResults = uint64(len(tx.Results))
assetID = tx.Inputs[index].AssetID()
assetIDBytes = assetID[:]
amount = tx.Inputs[index].Amount()
inputRefDataHash = hashData(tx.Inputs[index].ReferenceData).Bytes()
txRefDataHash = hashData(tx.ReferenceData).Bytes()
)

checkOutput := func(index uint64, refdatahash []byte, amount uint64, assetID []byte, vmVersion uint64, code []byte) (bool, error) {
if index >= uint64(len(tx.Outputs)) {
return false, vm.ErrBadValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope for this PR (because it would be a spec change), but it seems strange to me that this one check should abort the whole vm while all the other checks just return false. If my program asks whether there's an output at position 5, and there are only two outputs, the answer is "no, there isn't".

So yeah, it would be great (IMO) if this function could be a simple predicate. But since it can't, it might look a little cleaner to have it return error that's one of vm.ErrBadValue or vm.NotFound or nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, now that you mention it, the spec is silent on what to when index is out of range. (And the updated spec on the txgraph-spec branch is similarly silent but includes outdated language about "the number of outputs.")

I agree that just returning false would be preferable. @danrobinson @oleganza would this be acceptable, and if so will you clarify the spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

The spec says

Fails if the number of outputs is less or equal to index.

which I interpret to explicitly require the current behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line says that CHECKOUTPUT should fail execution if asked to find out-of-bounds output:

  1. Fails if the number of outputs is less or equal to index.

Same as in txgraph spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bobg i guess "outputs" should be "destinations" and moved into appropriate part (under "if we are in the Mux").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the spec is silent

Fails if the number of outputs is less or equal to index.

😊

Carry on...

Copy link
Contributor

@kr kr Mar 23, 2017

Choose a reason for hiding this comment

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

Did you have thoughts on my actual suggestion above?

But since it can't [be a simple predicate], it might look a little cleaner to have it return error that's one of vm.ErrBadValue or vm.NotFound or nil.

}
o := tx.Outputs[index]
if o.AssetVersion != 1 {
return false, nil
}
if o.Amount != uint64(amount) {
return false, nil
}
if o.VMVersion != uint64(vmVersion) {
return false, nil
}
if !bytes.Equal(o.ControlProgram, code) {
return false, nil
}
if !bytes.Equal(o.AssetID[:], assetID) {
return false, nil
}
if len(refdatahash) > 0 {
h := hashData(o.ReferenceData)
if !bytes.Equal(h[:], refdatahash) {
return false, nil
}
}
return true, nil
}

result := &vm.Context{
VMVersion: prog.VMVersion,
Code: prog.Code,
Arguments: args,

TxVersion: &tx.Version,

TxSigHash: &txSigHash,
NumResults: &numResults,
AssetID: &assetIDBytes,
Amount: &amount,
MinTimeMS: &tx.MinTime,
MaxTimeMS: &tx.MaxTime,
InputRefDataHash: &inputRefDataHash,
TxRefDataHash: &txRefDataHash,
InputIndex: &index,
CheckOutput: checkOutput,
}
switch inp := tx.Inputs[index].TypedInput.(type) {
case *IssuanceInput:
result.Nonce = &inp.Nonce
case *SpendInput:
spentOutputID := tx.TxHashes.SpentOutputIDs[index][:]
result.SpentOutputID = &spentOutputID
}

return result
}
2 changes: 1 addition & 1 deletion protocol/validation/block.go
Expand Up @@ -34,7 +34,7 @@ var (
// then calls ValidateBlock.
func ValidateBlockForAccept(ctx context.Context, snapshot *state.Snapshot, initialBlockHash bc.Hash, prevBlock, block *bc.Block, validateTx func(*bc.Tx) error) error {
if prevBlock != nil {
err := vm.VerifyBlockHeader(&prevBlock.BlockHeader, block)
err := vm.Verify(bc.NewBlockVMContext(block, prevBlock.ConsensusProgram, block.Witness))
if err != nil {
pkScriptStr, _ := vm.Disassemble(prevBlock.ConsensusProgram)
witnessStrs := make([]string, 0, len(block.Witness))
Expand Down
16 changes: 14 additions & 2 deletions protocol/validation/tx.go
Expand Up @@ -249,8 +249,20 @@ func CheckTxWellFormed(tx *bc.Tx) error {
}
}

for i := range tx.Inputs {
err := vm.VerifyTxInput(tx, uint32(i))
for i, inp := range tx.Inputs {
var (
prog bc.Program
args [][]byte
)
switch inp := inp.TypedInput.(type) {
case *bc.IssuanceInput:
prog = bc.Program{VMVersion: inp.VMVersion, Code: inp.IssuanceProgram}
args = inp.Arguments
case *bc.SpendInput:
prog = bc.Program{VMVersion: inp.VMVersion, Code: inp.ControlProgram}
args = inp.Arguments
}
err := vm.Verify(bc.NewTxVMContext(tx, uint32(i), prog, args))
if err != nil {
return badTxErrf(err, "validation failed in script execution, input %d", i)
}
Expand Down
8 changes: 8 additions & 0 deletions protocol/vm/assemble_test.go
Expand Up @@ -72,3 +72,11 @@ func TestDisassemble(t *testing.T) {
}
}
}

func mustDecodeHex(h string) []byte {
bits, err := hex.DecodeString(h)
if err != nil {
panic(err)
}
return bits
}
43 changes: 43 additions & 0 deletions protocol/vm/context.go
@@ -0,0 +1,43 @@
package vm

// Context contains the execution context for the virtual machine.
//
// Most fields are pointers and are not required to be present in all
// cases. A nil pointer means the value is absent in that context. If
// an opcode executes that requires an absent field to be present, it
// will return ErrContext.
//
// By convention, variables of this type have the name context, _not_
// ctx (to avoid confusion with context.Context).
type Context struct {
VMVersion uint64
Code []byte
Arguments [][]byte

// TxVersion must be present when verifying transaction components
// (such as spends and issuances).
TxVersion *uint64

// These fields must be present when verifying block headers.

BlockHash *[]byte
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to document how the optional fields work, maybe in the godoc for type Context or func Verify. Something brief would be fine, e.g. something like what I wrote in an earlier comment:

// Fields below this point are optional. If the corresponding opcode
// executes on a nil field, Verify will return ErrContext.

BlockTimeMS *uint64
NextConsensusProgram *[]byte

// Fields below this point are required by particular opcodes when
// verifying transaction components.

TxSigHash *[]byte
Copy link
Contributor

Choose a reason for hiding this comment

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

A slice is capable of being nil on its own, so if you want, the slice fields here don't have to be pointers. (Same as for the func field).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The slices that are hashes could be nil to mean "absent," because we know they should be non-empty if present. But other slices in this struct could be "empty and present," so those do need to be pointers to slices. My (weak) aesthetic preference was to handle all the slices that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a difference between a nil slice (which would mean absent) and a zero-length but non-nil slice (which would mean empty and present). So they don't need to be pointers to slices. But I don't feel strongly about it, I just wanted to raise the possibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good point. But, I don't know, I don't much like the idea of relying on the difference between []byte(nil) and []byte{} because I've seen one turn into the other too easily. Maybe just pilot error, but still.

NumResults *uint64
AssetID *[]byte
Amount *uint64
MinTimeMS *uint64
MaxTimeMS *uint64
InputRefDataHash *[]byte
TxRefDataHash *[]byte
InputIndex *uint32
Nonce *[]byte
SpentOutputID *[]byte

CheckOutput func(index uint64, data []byte, amount uint64, assetID []byte, vmVersion uint64, code []byte) (bool, error)
}
13 changes: 5 additions & 8 deletions protocol/vm/control.go
Expand Up @@ -61,14 +61,11 @@ func opCheckPredicate(vm *virtualMachine) error {
}

childVM := virtualMachine{
mainprog: vm.mainprog,
program: predicate,
runLimit: limit,
depth: vm.depth + 1,
dataStack: append([][]byte{}, vm.dataStack[l-n:]...),
tx: vm.tx,
txContext: vm.txContext,
inputIndex: vm.inputIndex,
context: vm.context,
program: predicate,
runLimit: limit,
depth: vm.depth + 1,
dataStack: append([][]byte{}, vm.dataStack[l-n:]...),
}
vm.dataStack = vm.dataStack[:l-n]

Expand Down
20 changes: 9 additions & 11 deletions protocol/vm/crypto.go
Expand Up @@ -127,25 +127,23 @@ func opCheckMultiSig(vm *virtualMachine) error {
}

func opTxSigHash(vm *virtualMachine) error {
if vm.tx == nil {
return ErrContext
}
err := vm.applyCost(256)
if err != nil {
return err
}
h := vm.txContext.TxSigHash
return vm.push(h[:], false)
if vm.context.TxSigHash == nil {
return ErrContext
}
return vm.push(*vm.context.TxSigHash, false)
}

func opBlockHash(vm *virtualMachine) error {
if vm.block == nil {
return ErrContext
}
h := vm.block.Hash()
err := vm.applyCost(4 * int64(len(h)))
err := vm.applyCost(1)
if err != nil {
return err
}
return vm.push(h[:], false)
if vm.context.BlockHash == nil {
return ErrContext
}
return vm.push(*vm.context.BlockHash, false)
}