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
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
93fd5e5
Remove vm->bc dependency.
df6946c
s/VMContext/Context/, and change from an interface with two implement…
4b58b74
Document the optional fields of a vm.Context.
a0f1323
Remove an unrelated change; it will go into its own PR.
7bfd324
auto rev id
iampogo File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
|
||
public final class RevId { | ||
public final String Id = "main/rev2841"; | ||
public final String Id = "main/rev2842"; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
package rev | ||
|
||
const ID string = "main/rev2841" | ||
const ID string = "main/rev2842" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
|
||
export const rev_id = "main/rev2841" | ||
export const rev_id = "main/rev2842" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
|
||
module Chain::Rev | ||
ID = "main/rev2841".freeze | ||
ID = "main/rev2842".freeze | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
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 | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
BlockTimeMS *uint64 | ||
NextConsensusProgram *[]byte | ||
|
||
// Fields below this point are required by particular opcodes when | ||
// verifying transaction components. | ||
|
||
TxSigHash *[]byte | ||
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) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 ofvm.ErrBadValue
orvm.NotFound
ornil
.There was a problem hiding this comment.
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 thetxgraph-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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec says
which I interpret to explicitly require the current behavior.
There was a problem hiding this comment.
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:
Same as in txgraph spec.
There was a problem hiding this comment.
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").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😊
Carry on...
There was a problem hiding this comment.
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?