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

Add decoder for FlowFees.FeesDeducted event #6304

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

illia-malachyn
Copy link
Contributor

@illia-malachyn illia-malachyn commented Aug 8, 2024

This will be used in flow EVM gateway and this is the place where we put such decoders before

@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 42.51%. Comparing base (400bcd0) to head (df3bd19).
Report is 858 commits behind head on master.

Files with missing lines Patch % Lines
fvm/evm/events/events.go 20.00% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6304      +/-   ##
==========================================
+ Coverage   41.50%   42.51%   +1.01%     
==========================================
  Files        2013     2066      +53     
  Lines      143577   191577   +48000     
==========================================
+ Hits        59590    81450   +21860     
- Misses      77813   103456   +25643     
- Partials     6174     6671     +497     
Flag Coverage Δ
unittests 42.51% <20.00%> (+1.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@janezpodhostnik
Copy link
Contributor

Can you also add a test please?

@illia-malachyn
Copy link
Contributor Author

illia-malachyn commented Aug 13, 2024

@janezpodhostnik, I'm a bit confused about how to approach the test.
I wanted to see if it works well by calling it in evm_test like DecodeFLOWTokensWithdrawnEventPayload but this event (speaking of FeesDeducted) is only called when transaction fees are enabled (they are turned off in unit tests).

I can write a simple test that creates such an event structure and decodes it. Would it be any useful doing it?

e.g.

	t.Run("FlowFees.FeesDeducted event decoded successfully", func(t *testing.T) {
		event := &events.FeesDeductedEventPayload{
			Amount:          cadence.UFix64(10000),
			InclusionEffort: cadence.UFix64(110000),
			ExecutionEffort: cadence.UFix64(12000),
		}

		cadenceEvent, err := event.ToCadence(flow.Emulator)
		require.NoError(t, err)

		payload, err := events.DecodeFeesDeductedEventPayload(cadenceEvent)
		require.NoError(t, err)

		assert.Equal(t, payload.Amount, event.Amount)
		assert.Equal(t, payload.InclusionEffort, event.InclusionEffort)
		assert.Equal(t, payload.ExecutionEffort, event.ExecutionEffort)
	})

where ToCadence looks like this:

func (p *FeesDeductedEventPayload) ToCadence(chainID flow.ChainID) (cadence.Event, error) {
   sc := systemcontracts.SystemContractsForChain(chainID)

   eventType := cadence.NewEventType(
   	sc.FlowFees.Location(),
   	"FlowFees.FeesDeducted",
   	[]cadence.Field{
   		{
   			Identifier: "amount",
   			Type:       cadence.UFix64Type,
   		},
   		{
   			Identifier: "inclusionEffort",
   			Type:       cadence.UFix64Type,
   		},
   		{
   			Identifier: "executionEffort",
   			Type:       cadence.UFix64Type,
   		},
   	},
   	nil)

   return cadence.NewEvent([]cadence.Value{
   	p.Amount,
   	p.InclusionEffort,
   	p.ExecutionEffort,
   }).WithType(eventType), nil
}

@janezpodhostnik
Copy link
Contributor

Yeah initially I had something like what you described in mind, but you are correct that that would basically not be testing much.

@github-actions github-actions bot added the Stale Label used when marking an issue stale. label Oct 26, 2024
@github-actions github-actions bot closed this Nov 2, 2024
@franklywatson franklywatson reopened this Nov 2, 2024
@github-actions github-actions bot removed the Stale Label used when marking an issue stale. label Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants