Adding event limit per transaction #128
Conversation
…go into ramtin/adding-event-limiter
ramtinms
changed the title
…go into ramtin/adding-event-limiter
…go into ramtin/adding-event-limiter
…go into ramtin/adding-event-limiter
…go into ramtin/adding-event-limiter
| if err != nil { | ||
| return nil, errors.New("error decoding events") | ||
| } | ||
| addr = flow.Address(data.(cadence.Event).Fields[0].(cadence.Address)) |
There was a problem hiding this comment.
Use flow.AccountCreatedEvent(event).Address() instead
There was a problem hiding this comment.
AccountCreatedEvent doesn't exist in flow, I guess flowsdk has some functionality
There was a problem hiding this comment.
hmm, I can't add it to models, it adds circular dependencies
There was a problem hiding this comment.
Ah, I thought this was declared in the models, nevermind then
| address := flow.Address(tx.Events[0].Fields[0].(cadence.Address)) | ||
| data, err := jsoncdc.Decode(tx.Events[0].Payload) | ||
| require.NoError(t, err) | ||
| address := flow.Address(data.(cadence.Event).Fields[0].(cadence.Address)) |
There was a problem hiding this comment.
Use flow.AccountCreatedEvent(event).Address() instead here, and in the other cases below
|
|
||
| func (e *hostEnv) GenerateUUID() uint64 { | ||
| uuid, err := e.uuidGenerator.GenerateUUID() | ||
| payload, err := jsoncdc.Encode(event) |
There was a problem hiding this comment.
Should we really measure size by encoding to the very verbose JSON format?
There was a problem hiding this comment.
Unfortunately, that's how we store them right now and deliver them to users, so I kept the JSON encoding so users can have some measures of how big the events are. We increase the limit in the future when we store things more binary.
There was a problem hiding this comment.
Oh, I thought we store them as Cadence interpreter values, like in the account storage. It would be great if we could switch, I don't even think the JSON encoding is deterministic? But that's for another PR - If we store events as JSON, then we should measure it as that 👍
There was a problem hiding this comment.
I don't even think the JSON encoding is deterministic
thats a good point, we def need to change for event verification
This PR introduces these changes
Steps remaining