From 2aa92cb4f85e4625a2c58f52b551245092acd3b9 Mon Sep 17 00:00:00 2001 From: Ben Guidarelli Date: Fri, 10 Feb 2023 14:48:57 -0500 Subject: [PATCH 1/2] use StackType to check assignability --- data/transactions/logic/assembler.go | 14 +++----------- data/transactions/logic/eval.go | 17 +++++++++++++++++ data/transactions/logic/evalStateful_test.go | 2 +- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 8bf1529d01..6b7be18367 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1693,16 +1693,8 @@ func (le lineError) Unwrap() error { return le.Err } -func typecheck(expected, got avmType) bool { - // Some ops push 'any' and we wait for run time to see what it is. - // Some of those 'any' are based on fields that we _could_ know now but haven't written a more detailed system of typecheck for (yet). - if expected == avmAny && got == avmNone { // Any is lenient, but stack can't be empty - return false - } - if (expected == avmAny) || (got == avmAny) { - return true - } - return expected == got +func typecheck(expected, got StackType) bool { + return got.AssignableTo(expected) } // newline not included since handled in scanner @@ -1827,7 +1819,7 @@ func (ops *OpStream) trackStack(args StackTypes, returns StackTypes, instruction } else { ops.trace(", %s", argType) } - if !typecheck(argType.AVMType, stype.AVMType) { + if !typecheck(argType, stype) { ops.typeErrorf("%s arg %d wanted type %s got %s", strings.Join(instruction, " "), i, argType, stype) } diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 86263c3226..e2ea1bc35c 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -632,6 +632,23 @@ func (at avmType) String() string { return "internal error, unknown type" } +// stackType lifts the AVM type to a StackType +// it can do this because the base StackTypes +// are a superset of avmType +func (at avmType) stackType() StackType { + switch at { + case avmNone: + return StackNone + case avmAny: + return StackAny + case avmUint64: + return StackUint64 + case avmBytes: + return StackBytes + } + return StackNone +} + var ( // StackUint64 is any valid uint64 StackUint64 = NewStackType(avmUint64, bound(0, math.MaxUint64)) diff --git a/data/transactions/logic/evalStateful_test.go b/data/transactions/logic/evalStateful_test.go index 638cd8e358..48a24f2795 100644 --- a/data/transactions/logic/evalStateful_test.go +++ b/data/transactions/logic/evalStateful_test.go @@ -2577,7 +2577,7 @@ func TestReturnTypes(t *testing.T) { stackType := cx.stack[i].argType() retType := spec.Return.Types[i] require.True( - t, typecheck(retType.AVMType, stackType), + t, typecheck(retType, stackType.stackType()), "%s expected to return %s but actual is %s", spec.Name, retType, stackType, ) } From b966f21f7a85781b2503d78658f60d95fe8fa978 Mon Sep 17 00:00:00 2001 From: Ben Guidarelli Date: Fri, 10 Feb 2023 15:07:51 -0500 Subject: [PATCH 2/2] adding type refinement to bytes and bzero --- data/transactions/logic/assembler.go | 16 +++++++++++++++- data/transactions/logic/assembler_test.go | 15 ++++++++------- data/transactions/logic/eval.go | 2 +- data/transactions/logic/evalAppTxn_test.go | 12 ++++++------ data/transactions/logic/eval_test.go | 2 +- data/transactions/logic/opcodes.go | 2 +- 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 6b7be18367..90fbe18c2c 100644 --- a/data/transactions/logic/assembler.go +++ b/data/transactions/logic/assembler.go @@ -1506,6 +1506,20 @@ func typePushInts(pgm *ProgramKnowledge, args []string) (StackTypes, StackTypes, return nil, types, nil } +func typeBzero(pgm *ProgramKnowledge, args []string) (StackTypes, StackTypes, error) { + // Bzero should only allow its input int to be up to maxStringSize bytes + return StackTypes{StackUint64.narrowed(bound(0, maxStringSize))}, StackTypes{StackBytes}, nil +} + +func typeByte(pgm *ProgramKnowledge, args []string) (StackTypes, StackTypes, error) { + if len(args) == 0 { + return nil, StackTypes{StackBytes}, nil + } + val, _, _ := parseBinaryArgs(args) + l := uint64(len(val)) + return nil, StackTypes{NewStackType(avmBytes, static(l), fmt.Sprintf("[%d]byte", l))}, nil +} + func joinIntsOnOr(singularTerminator string, list ...int) string { if len(list) == 1 { switch list[0] { @@ -1587,7 +1601,7 @@ const anyImmediates = -1 var pseudoOps = map[string]map[int]OpSpec{ "int": {anyImmediates: OpSpec{Name: "int", Proto: proto(":i"), OpDetails: assembler(asmInt)}}, - "byte": {anyImmediates: OpSpec{Name: "byte", Proto: proto(":b"), OpDetails: assembler(asmByte)}}, + "byte": {anyImmediates: OpSpec{Name: "byte", Proto: proto(":b"), OpDetails: assembler(asmByte).typed(typeByte)}}, // parse basics.Address, actually just another []byte constant "addr": {anyImmediates: OpSpec{Name: "addr", Proto: proto(":b"), OpDetails: assembler(asmAddr)}}, // take a signature, hash it, and take first 4 bytes, actually just another []byte constant diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index 64512be95e..0a599e4dda 100644 --- a/data/transactions/logic/assembler_test.go +++ b/data/transactions/logic/assembler_test.go @@ -772,7 +772,7 @@ int 1 + // comment ` - testProg(t, source, AssemblerMaxVersion, Expect{3, "+ arg 0 wanted type uint64 got []byte"}) + testProg(t, source, AssemblerMaxVersion, Expect{3, "+ arg 0 wanted type uint64 got [5]byte"}) } // mutateProgVersion replaces version (first two symbols) in hex-encoded program @@ -1853,7 +1853,7 @@ balance int 1 ==` for v := uint64(2); v < directRefEnabledVersion; v++ { - testProg(t, source, v, Expect{2, "balance arg 0 wanted type uint64 got []byte"}) + testProg(t, source, v, Expect{2, "balance arg 0 wanted type uint64 got [1]byte"}) } for v := uint64(directRefEnabledVersion); v <= AssemblerMaxVersion; v++ { testProg(t, source, v) @@ -1869,7 +1869,7 @@ min_balance int 1 ==` for v := uint64(3); v < directRefEnabledVersion; v++ { - testProg(t, source, v, Expect{2, "min_balance arg 0 wanted type uint64 got []byte"}) + testProg(t, source, v, Expect{2, "min_balance arg 0 wanted type uint64 got [1]byte"}) } for v := uint64(directRefEnabledVersion); v <= AssemblerMaxVersion; v++ { testProg(t, source, v) @@ -2656,10 +2656,11 @@ func TestTxTypes(t *testing.T) { t.Parallel() testProg(t, "itxn_begin; itxn_field Sender", 5, Expect{1, "itxn_field Sender expects 1 stack argument..."}) testProg(t, "itxn_begin; int 1; itxn_field Sender", 5, Expect{1, "...wanted type addr got uint64"}) - testProg(t, "itxn_begin; byte 0x56127823; itxn_field Sender", 5) + testProg(t, "itxn_begin; byte 0x56127823; itxn_field Sender", 5, Expect{1, "...wanted type addr got [4]byte"}) + testProg(t, "itxn_begin; global ZeroAddress; itxn_field Sender", 5) testProg(t, "itxn_begin; itxn_field Amount", 5, Expect{1, "itxn_field Amount expects 1 stack argument..."}) - testProg(t, "itxn_begin; byte 0x87123376; itxn_field Amount", 5, Expect{1, "...wanted type uint64 got []byte"}) + testProg(t, "itxn_begin; byte 0x87123376; itxn_field Amount", 5, Expect{1, "...wanted type uint64 got [4]byte"}) testProg(t, "itxn_begin; int 1; itxn_field Amount", 5) } @@ -2671,14 +2672,14 @@ func TestBadInnerFields(t *testing.T) { testProg(t, "itxn_begin; int 1000; itxn_field LastValid", 5, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; int 32; bzero; itxn_field Lease", 5, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; byte 0x7263; itxn_field Note", 5, Expect{1, "...Note field was introduced in v6..."}) - testProg(t, "itxn_begin; byte 0x7263; itxn_field VotePK", 5, Expect{1, "...VotePK field was introduced in v6..."}) + testProg(t, "itxn_begin; global ZeroAddress; itxn_field VotePK", 5, Expect{1, "...VotePK field was introduced in v6..."}) testProg(t, "itxn_begin; int 32; bzero; itxn_field TxID", 5, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; int 1000; itxn_field FirstValid", 6, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; int 1000; itxn_field LastValid", 6, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; int 32; bzero; itxn_field Lease", 6, Expect{1, "...is not allowed."}) testProg(t, "itxn_begin; byte 0x7263; itxn_field Note", 6) - testProg(t, "itxn_begin; byte 0x7263; itxn_field VotePK", 6) + testProg(t, "itxn_begin; global ZeroAddress; itxn_field VotePK", 6) testProg(t, "itxn_begin; int 32; bzero; itxn_field TxID", 6, Expect{1, "...is not allowed."}) } diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index e2ea1bc35c..c63ca6d7a3 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -632,7 +632,7 @@ func (at avmType) String() string { return "internal error, unknown type" } -// stackType lifts the AVM type to a StackType +// stackType lifts the avmType to a StackType // it can do this because the base StackTypes // are a superset of avmType func (at avmType) stackType() StackType { diff --git a/data/transactions/logic/evalAppTxn_test.go b/data/transactions/logic/evalAppTxn_test.go index b33a97d742..64b0f9b3bc 100644 --- a/data/transactions/logic/evalAppTxn_test.go +++ b/data/transactions/logic/evalAppTxn_test.go @@ -104,15 +104,15 @@ func TestFieldTypes(t *testing.T) { t.Parallel() ep, _, _ := MakeSampleEnv() - TestApp(t, "itxn_begin; byte \"pay\"; itxn_field Sender;", ep, "not an address") + // Use NoTrack to skip assembly errors + TestApp(t, NoTrack("itxn_begin; byte \"pay\"; itxn_field Sender;"), ep, "not an address") TestApp(t, NoTrack("itxn_begin; int 7; itxn_field Receiver;"), ep, "not an address") - TestApp(t, "itxn_begin; byte \"\"; itxn_field CloseRemainderTo;", ep, "not an address") - TestApp(t, "itxn_begin; byte \"\"; itxn_field AssetSender;", ep, "not an address") - // can't really tell if it's an addres, so 32 bytes gets further - TestApp(t, "itxn_begin; byte \"01234567890123456789012345678901\"; itxn_field AssetReceiver;", + TestApp(t, NoTrack("itxn_begin; byte \"\"; itxn_field CloseRemainderTo;"), ep, "not an address") + TestApp(t, NoTrack("itxn_begin; byte \"\"; itxn_field AssetSender;"), ep, "not an address") + TestApp(t, NoTrack("itxn_begin; byte \"01234567890123456789012345678901\"; itxn_field AssetReceiver;"), ep, "invalid Account reference") // but a b32 string rep is not an account - TestApp(t, "itxn_begin; byte \"GAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYZIZD42E\"; itxn_field AssetCloseTo;", + TestApp(t, NoTrack("itxn_begin; byte \"GAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYZIZD42E\"; itxn_field AssetCloseTo;"), ep, "not an address") TestApp(t, NoTrack("itxn_begin; byte \"pay\"; itxn_field Fee;"), ep, "not a uint64") diff --git a/data/transactions/logic/eval_test.go b/data/transactions/logic/eval_test.go index b89a6cd0da..26520b6c11 100644 --- a/data/transactions/logic/eval_test.go +++ b/data/transactions/logic/eval_test.go @@ -4502,7 +4502,7 @@ func TestBits(t *testing.T) { testAccepts(t, "byte 0xfffff0; int 21; int 1; setbit; byte 0xfffff4; ==", 3) testAccepts(t, "byte 0xfffff4; int 1; int 0; setbit; byte 0xbffff4; ==", 3) - testPanics(t, "byte 0xfffff4; int 24; int 0; setbit; byte 0xbf; ==", 3) + testPanics(t, "byte 0xfffff4; int 24; int 0; setbit; byte 0xbffff4; ==", 3) testAccepts(t, "byte 0x0000; int 3; int 1; setbit; byte 0x1000; ==", 3) testAccepts(t, "byte 0x0000; int 15; int 1; setbit; byte 0x0001; ==", 3) diff --git a/data/transactions/logic/opcodes.go b/data/transactions/logic/opcodes.go index ff0f9a2dd6..db4ec96dda 100644 --- a/data/transactions/logic/opcodes.go +++ b/data/transactions/logic/opcodes.go @@ -604,7 +604,7 @@ var OpSpecs = []OpSpec{ {0xac, "b&", opBytesBitAnd, proto("bb:b"), 4, costly(6)}, {0xad, "b^", opBytesBitXor, proto("bb:b"), 4, costly(6)}, {0xae, "b~", opBytesBitNot, proto("b:b"), 4, costly(4)}, - {0xaf, "bzero", opBytesZero, proto("i:b"), 4, detDefault()}, + {0xaf, "bzero", opBytesZero, proto("i:b"), 4, detDefault().typed(typeBzero)}, // AVM "effects" {0xb0, "log", opLog, proto("b:"), 5, only(ModeApp)},